Repository navigation
[ISSUE #11481] fix(broker): do not abort the queue-mapping clean pass on a missing remote queue - #11482
Merged
RongtongJin merged 1 commit intoOct 23, 2026
Conversation
…n pass on a missing remote queue cleanItemListMoreThanSecondGen() validates, for every queue whose leader moved, that the new leader agrees. It reads the new leader's hosted queues with get(qId) and calls isEmpty() on the result unconditionally: when the remote broker's mapping does not contain that queue id at all — the transient state this service exists to reconcile — the read returns null and the pass dies with a NullPointerException. The exception is swallowed by the per-topic catch, so the visible damage is silent: every remaining queue of that topic is skipped for this cycle, the mapping is left half-cleaned, and the failure repeats every five minutes. Treat a missing remote queue as "not confirmed yet" and continue, which is how the surrounding reads already handle absent data. Red/green: remoteLeaderMissingTheQueueIdDoesNotAbortTheRestOfTheTopic fails on the old code (the second queue is never removed because the NPE aborts the pass) and passes with the null guard. Broker module: 815 tests, the 7 failures are pre-existing (RocksDB/filter/schedule/pop-lite/ BrokerOuterAPI) and reproduce identically with this change stashed.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #11482 +/- ##
=============================================
- Coverage 49.79% 49.75% -0.05%
+ Complexity 14387 14366 -21
=============================================
Files 1390 1390
Lines 103264 103264
Branches 13504 13504
=============================================
- Hits 51419 51377 -42
- Misses 45657 45675 +18
- Partials 6188 6212 +24 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
RongtongJin
approved these changes
Oct 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #11481
cleanItemListMoreThanSecondGen()reads the new leader's mapping withhostedQueues.get(qId)and callsisEmpty()without a null check. When the remote mapping has no entry for that queue id — the transient state this service is there to reconcile — the read returnsnulland the pass dies with aNullPointerException, which the per-topiccatch (Throwable)swallows. The result is silent and sticky: the rest of that topic's queues are skipped for this cycle, the mapping stays half-cleaned, and the error repeats every five minutes.The change is one guard — a missing remote queue is treated like every other "not confirmed yet" case around it:
Red/green:
remoteLeaderMissingTheQueueIdDoesNotAbortTheRestOfTheTopicbuilds a topic with two moved queues; the new leader's mapping confirms queue 1 but has no entry for queue 0. On the old code the NPE aborts the pass and queue 1 is never removed (test fails); with the guard queue 1 is removed and queue 0 is left alone (test passes). The existing five tests in the class stay green.Broker module: 815 tests, the 7 failures (RocksDB transfer, store filter, schedule, pop-lite, BrokerOuterAPI) are pre-existing — stashing this change reproduces the exact same set, and all live in unrelated areas.
No coordination needed: no open PR touches this file; the only related closed item is #8728 (test coverage enhancement, already resolved).