[ISSUE #10698] Restore atomic producer group registration in ProducerManager

[ai-yang]

Which Issue(s) This PR Fixes

• Fixes [Bug] ProducerManager new-group registration can be detached by concurrent inactive-channel scan #10698

Brief Description

Restore atomic publication of a newly registered producer group without changing the registration-gate or fast-channel behavior introduced later.

• Pre-populate a candidate group map with its first channel before publishing it.
• Fall back to putIfAbsent on the winning map when another registration creates the group first.
• Execute client-id indexing, connection logging, and fast-channel attributes only for the thread that inserts a new group/channel mapping.
• Refresh the winner's existing ClientChannelInfo timestamp for duplicate registrations.

Root Cause

registerProducer() published an empty inner map to groupChannelTable. A concurrent inactive-channel scan could remove the empty group and emit GROUP_UNREGISTER before registration inserted its first channel, leaving the channel and client-id mappings detached from the producer-group index.

This is a regression of #8846 / #8847 after #9293 restored the empty-map-first ordering while adding registration gating and fast-channel processing.

Impact

New producer groups cannot disappear during concurrent scanning, and the group/channel/client-id indexes remain consistent. The current registration switch truth table, timestamp refresh, statistics, and listener semantics are preserved. This PR intentionally does not expand into broader synchronization for already-existing empty groups.

How Did You Test This Change?

• Deterministic red test on the previous implementation: the scanner removed FooBar while registration was blocked at the Channel's first hashCode(), and the group assertion failed.
• ProducerManagerTest: 9 tests passed, including latch-controlled concurrent scanning and fast-channel attribute coverage.
• Full Broker test suite: 754 tests passed, 0 failures, 0 errors, 4 skipped.
• Reactor dependency build/install: 10 modules succeeded.
• Maven Checkstyle: 0 violations.
• git diff --check.

[RockteMQ-AI]

Review by github-manager-bot

Summary

This PR fixes a regression in ProducerManager.registerProducer() where the group's inner map was published empty before the first channel was inserted. A concurrent inactive-channel scan could remove the empty group and emit GROUP_UNREGISTER before registration completed, leaving the channel and client-id mappings orphaned. The fix pre-populates the candidate map with the first channel before atomic publication.

Findings

• [Positive] ProducerManager.java:236-247 — Pre-populating newChannelTable with the first channel before putIfAbsent is the correct fix. This eliminates the window where the scanner could see an empty group map.

• [Positive] The newChannel flag cleanly separates the "first channel in group" path from the "refresh existing" path. The logic correctly handles both the race where we win the putIfAbsent and where we lose it.

• [Positive] When we lose the putIfAbsent race (prev != null), the code correctly falls through to the !newChannel block which does putIfAbsent on the winner's map. This ensures our channel is still registered even if another thread created the group first.

• [Info] ProducerManager.java:250-252 — The newChannel flag is reused for both group-level and channel-level "newness". While functionally correct (in both cases "new" means "this channel wasn't in the table before"), a reader might need a moment to trace the two assignment sites. Consider a brief inline comment at line 251 explaining that this is the channel-level check after the group-level check above.

• [Positive] The timestamp refresh in the else branch (clientChannelInfoFound != null case) preserves the original behavior for duplicate registrations without triggering the "new producer" log or fast-channel attribute addition.

• [Positive] Test coverage includes a latch-controlled concurrent scanning test that deterministically reproduces the race on the old code. Full broker suite (754 tests) passes.

Suggestions

1. The PR description mentions this is a regression of #8846/#8847 after #9293. Consider adding a // Regression: see #8846, #9293 comment near the putIfAbsent block to help future bisectors.

2. The clientChannelInfoFound = null initialization at line 209 is now always assigned before use (either in the !newChannel block or implicitly via newChannel = true). This is good defensive coding.

Verdict

Clean, well-scoped fix for a real regression. The atomic publication pattern is correct and the test coverage is solid.

---

Automated review by github-manager-bot

[RockteMQ-AI]

Summary

Defensive fix with proper validation and test coverage. LGTM.

---

Automated review by github-manager-bot

[ai-yang]

Refreshed this PR onto the current develop (e348efa66) and force-pushed one signed-off commit (cd6454ea4).

• ProducerManagerTest: 9/9 passed, including the latch-controlled scanner/registration race;
• all 10 modules in the targeted broker -am reactor succeeded;
• Checkstyle and SpotBugs reported no findings;
• git diff --check passed.

The production change is +19/-12 lines; +82/-3 lines are regression tests. Full CI has been retriggered.

@xdkxlk @lizhanhui, could you please take a human review when convenient, particularly of the atomic publication and listener/index consistency in ProducerManager.registerProducer?

[codecov-commenter]

Codecov Report

❌ Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.53%. Comparing base (e348efa) to head (cd6454e).

Files with missing lines	Patch %	Lines
...apache/rocketmq/broker/client/ProducerManager.java	85.71%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10699      +/-   ##
=============================================
- Coverage      48.58%   48.53%   -0.06%     
+ Complexity     13678    13664      -14     
=============================================
  Files           1381     1381              
  Lines         101475   101482       +7     
  Branches       13190    13191       +1     
=============================================
- Hits           49304    49253      -51     
- Misses         46170    46209      +39     
- Partials        6001     6020      +19     

☔ 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.