[ISSUE #10580] Fix static topic epoch comparator overflow

[Aias00]

What changed

Fix static topic epoch ordering to avoid integer overflow when comparing mapping epoch values.

Two comparators previously subtracted long epoch values and cast the result to int:

(int) (o2.getValue().getEpoch() - o1.getValue().getEpoch())

and:

(int) (o2.getEpoch() - o1.getEpoch())

When the epoch difference exceeds Integer.MAX_VALUE, the comparator can overflow and process stale mapping metadata before newer metadata.

This PR replaces subtraction-based comparisons with Long.compare(...) in:

• ClientMetadata.topicRouteData2EndpointsForStaticTopic(...)
• TopicQueueMappingUtils.checkAndBuildMappingItems(...)

Fixes #10580

Why

Static topic mapping epochs fence old metadata. Ordering by epoch must remain correct even when epoch gaps are larger than Integer.MAX_VALUE, otherwise route building or duplicate queue replacement can prefer stale mapping data.

Tests

Added regression coverage for large epoch gaps:

• ClientMetadataTest: verifies routing resolves to the latest epoch broker mapping
• TopicQueueMappingUtilsTest: verifies duplicate global queue replacement keeps the latest epoch mapping

Verified locally:

mvn -q -pl remoting -DskipTests=false -Dtest=ClientMetadataTest,TopicQueueMappingUtilsTest -Djacoco.skip=true test
mvn -q -pl remoting -DskipTests compile -Dspotbugs.skip=true -Dcheckstyle.skip=true
mvn -q -DskipTests compile -Dspotbugs.skip=true -Dcheckstyle.skip=true
git diff --check

Notes

The change keeps the existing ordering semantics: higher epoch first. It only replaces unsafe subtraction with Long.compare(...).

[Copilot]

Pull request overview

Note

Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.

Fixes static-topic epoch ordering by replacing subtraction-based long comparisons (that could overflow when cast to int) with Long.compare(...), and adds regression tests to cover large epoch gaps.

Changes:

• Replace overflow-prone epoch comparator implementations with Long.compare(...) (higher epoch first).
• Add regression tests to ensure the latest-epoch mapping wins when epochs differ by more than Integer.MAX_VALUE.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
remoting/src/main/java/org/apache/rocketmq/remoting/rpc/ClientMetadata.java	Uses Long.compare to correctly sort mapping infos by epoch without overflow.
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/statictopic/TopicQueueMappingUtils.java	Uses Long.compare to correctly sort mapping details by epoch without overflow.
remoting/src/test/java/org/apache/rocketmq/remoting/rpc/ClientMetadataTest.java	Adds regression test for routing selection with very large epoch gaps.
remoting/src/test/java/org/apache/rocketmq/remoting/protocol/statictopic/TopicQueueMappingUtilsTest.java	Adds regression test ensuring duplicate replacement keeps the latest-epoch mapping.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 48.17%. Comparing base (8242c1e) to head (95fcbd6).
⚠️ Report is 3 commits behind head on develop.

Files with missing lines	Patch %	Lines
...g/apache/rocketmq/remoting/rpc/ClientMetadata.java	83.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10581      +/-   ##
=============================================
- Coverage      48.26%   48.17%   -0.09%     
+ Complexity     13433    13424       -9     
=============================================
  Files           1378     1378              
  Lines         100817   100824       +7     
  Branches       13040    13042       +2     
=============================================
- Hits           48660    48573      -87     
- Misses         46211    46293      +82     
- Partials        5946     5958      +12     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[RockteMQ-AI]

Review by github-manager-bot

Summary

Fixes integer overflow in static topic epoch comparators by replacing (int)(o2.getEpoch() - o1.getEpoch()) subtraction patterns with Comparator.comparingLong().reversed(), and tightens the epoch comparison logic in ClientMetadata to use strict > instead of >=.

Findings

• [Info] TopicQueueMappingUtils.java:342 — Comparator.comparingLong(TopicQueueMappingDetail::getEpoch).reversed() is the idiomatic and overflow-safe replacement. Clean fix.
• [Info] ClientMetadata.java:123 — The method reference cast (Map.Entry<String, TopicQueueMappingInfo> entry) is needed for type inference but adds verbosity. This is acceptable.
• [Warning] ClientMetadata.java:128-131 — The logic change from >= to > for info.getEpoch() >= maxTotalNumOfEpoch is a behavioral change beyond just fixing overflow. Previously, entries with equal epoch would still update maxTotalNums if they had more queues. Now, only strictly newer epochs update both trackers. This means for entries with the same epoch, only the first one encountered (in sorted order) will set maxTotalNumOfEpoch, and subsequent entries with the same epoch will fall into the else if branch. Please verify this is the intended semantics — the else if on the next line handles the equal-epoch case for maxTotalNums only, which seems correct but is a subtle change worth confirming.
• [Info] Tests cover the overflow scenario with Integer.MAX_VALUE + 1L epoch difference — good validation of the fix.

Suggestions

• Consider adding a brief comment in ClientMetadata explaining why the epoch comparison was changed from >= to >, to help future readers understand the intent.

---

Automated review by github-manager-bot