[Bug] ProducerManager new-group registration can be detached by concurrent inactive-channel scan
- Dominant language
- Java
- Stars
- 22.6k
- Forks
- 12k
- Avg merge
- 3d 1h
- Merged PRs (30d)
- 27
Description
### Affected baseline
`develop` at `00e45b8a6db23efbe756d0306f10716156cfd4dd`.
### Problem
`ProducerManager.registerProducer()` publishes a newly created producer-group entry before adding its first channel. Concurrent `scanNotActiveChannel()` can observe that empty inner map, remove the group, and emit `GROUP_UNREGISTER`. The registration thread then inserts the channel into the now-detached map and returns successfully.
The resulting indexes disagree:
- `clientChannelTable` contains `clientId -> channel`;
- the detached inner map contains `channel -> ClientChannelInfo`;
- `groupChannelTable` no longer contains the producer group.
Group lookup and transaction-producer selection therefore fail even though registration returned normally, and listeners receive a false group-unregister event.
### Deterministic interleaving
1. `registerProducer()` creates an empty `ConcurrentHashMap`.
2. It publishes that map with `groupChannelTable.putIfAbsent(group, channelTable)`.
3. Before the first channel lookup/insertion, `scanNotActiveChannel()` observes `chlMap.isEmpty()`, removes the outer entry, and emits `GROUP_UNREGISTER`.
4. Registration inserts into its detached map and updates `clientChannelTable`.
A regression test can pause the registering thread at the Channel's first `hashCode()` call with two latches. On current code that first hash occurs only after the empty outer entry has been published, making the scan/removal deterministic.
### Expected behavior
A newly published producer-group map must already contain its first channel. After registration completes:
- the group exists in `groupChannelTable`;
- its channel mapping exists;
- `findChannel(clientId)` returns the same channel;
- no spurious `GROUP_UNREGISTER` is emitted;
- fast-channel producer-group attributes and the existing registration-gate behavior remain unchanged.
### Root cause / regression history
This is a regression of [#8846](https://github.com/apache/rocketmq/issues/8846) and merged fix [#8847](https://github.com/apache/rocketmq/pull/8847), which pre-populated the candidate inner map before publishing it. [#9293](https://github.com/apache/rocketmq/pull/9293) later added producer-registration gating and fast-channel bookkeeping but restored the empty-map-first ordering. `ProducerManager.java` has not changed since #9293.
### Proposed scope
- Pre-populate a candidate inner map before publishing it to `groupChannelTable`.
- If another thread wins outer insertion, use `putIfAbsent` on the winning inner map.
- Use a `newChannel` flag so client-id mapping, logging, and the fast-channel attribute execute exactly once.
- Preserve the current registration-gate truth table, timestamp refresh, and registration-time statistics.
- Add a latch-controlled regression test plus fast-channel attribute coverage.
This issue is intentionally limited to the new-group empty-map publication regression. Broader synchronization for an already existing group becoming empty during concurrent lifecycle operations should be handled separately.
I am working on a focused fix and will submit a PR against `develop`.
Contributor guide
Research direction
Start with ProducerManager.java, especially registerProducer() and scanNotActiveChannel(), and trace groupChannelTable, clientChannelTable, and findChannel(clientId). Add the latch-controlled regression test and fast-channel attribute coverage described in the issue; done means registration leaves both indexes consistent, emits no spurious GROUP_UNREGISTER, and preserves the existing gate and statistics behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- backend, distributed-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 25/100