Skip to content

[ISSUE #10849] Fix static topic epoch sort overflow - #11186

Open
beautyarbutin wants to merge 1 commit into
apache:developfrom
beautyarbutin:fix/static-topic-epoch-order
Open

beautyarbutin wants to merge 1 commit into
apache:developfrom
beautyarbutin:fix/static-topic-epoch-order

Conversation

@beautyarbutin

Copy link
Copy Markdown

Which Issue(s) This PR Fixes

Brief Description

Static-topic epoch comparators in TopicQueueMappingUtils and ClientMetadata subtracted two long values and narrowed the result to int. Epoch deltas larger than Integer.MAX_VALUE could therefore reverse the intended newest-first order.

This change uses Long.compare in both ordering paths. It adds regression coverage for replacement mode selecting the latest broker mapping across the full positive long range and for client endpoint construction retaining the latest broker.

How Did You Test This Change?

  • Confirmed the new server-side regression test failed before the fix: expected latest but received stale.
  • Ran mvn -pl remoting -am -DskipITs -Dtest=TopicQueueMappingUtilsTest,ClientMetadataTest -Dsurefire.failIfNoSpecifiedTests=false test.
  • Result: 15 tests passed; Checkstyle reported 0 violations; SpotBugs reported no errors or warnings.

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Fixes integer overflow in epoch comparators used for static topic routing. The old (int)(o2.getEpoch() - o1.getEpoch()) pattern silently reverses sort order when the epoch delta exceeds Integer.MAX_VALUE. Replaced with Long.compare() in both TopicQueueMappingUtils and ClientMetadata.

Findings

  • [Critical] This is a correctness bug — in long-running clusters, epoch values can diverge enough to cause the comparator to return wrong ordering, potentially routing messages to stale brokers. Good fix.
  • [Info] Tests use Long.MAX_VALUE vs 0 as the extreme case, which correctly reproduces the overflow. Both test classes cover the respective comparator paths.

Overall

Minimal, correct fix with proper regression tests. LGTM.


Automated review by RockteMQ-AI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Static topic mapping may select a stale epoch when the epoch delta overflows

2 participants