ai-yang opened a new issue, #10700:
URL: https://github.com/apache/rocketmq/issues/10700
### Affected baseline
`develop` at `00e45b8a6db23efbe756d0306f10716156cfd4dd`.
### Problem
Concurrent first offset commits for the same `topic@group` and different
queue IDs can overwrite each other in both the classic JSON
`ConsumerOffsetManager` and the RocksDB v1 manager.
Both implementations currently follow this pattern:
1. Read `offsetTable.get(topic@group)` and receive `null`.
2. Each thread creates its own inner queue-offset map.
3. Each writes one queue to its private map.
4. Each unconditionally publishes with `offsetTable.put(key, map)`.
The last outer write replaces the other map, so one queue disappears from
memory and subsequent persistence.
RocksDB v1 has a second race in incremental mode. It serializes the entire
inner queue map as one RocksDB value. A thread can serialize an older `{q0}`
snapshot, pause, let another thread write `{q0,q1}`, then write the older batch
last. Memory still contains both queues, but a restart reloads only `q0`.
### Expected behavior
- Concurrent first commits for different queue IDs retain every queue in
memory and after persistence/reload.
- Classic and RocksDB v1 atomically initialize the shared inner map with
outer `putIfAbsent`.
- RocksDB v1 preserves its LMQ `ConcurrentHashMap<>(1, 1.0F)` initialization.
- Incremental persistence orders the update, version change, whole-map
serialization, and WAL write for the same `topic@group` while allowing
different keys to proceed concurrently.
- RocksDB v2 remains unchanged: it already uses outer `putIfAbsent` and
persists each queue under an independent RocksDB key.
### Root cause
The classic and v1 managers use check-then-act initialization rather than
atomically publishing one shared map. In v1 incremental mode, whole-map
read/modify/serialize/write cycles for the same RocksDB key have no shared
critical section.
### Deterministic test plan
- Use a barrier-backed outer map whose first two `get(key)` calls both
return their saved `null`; commit two queue IDs concurrently and verify classic
JSON encode/decode retains both.
- Repeat against RocksDB v1 periodic persistence and verify `persist -> stop
-> clear -> load` retains both queues.
- In incremental mode, pause the first `batchPutWithWal` after its old
snapshot has been serialized. Let the second commit either write ahead on
current code or block on the fixed per-key monitor, then reload and verify both
queues remain.
### Related work and scope
Historical unmerged [PR #1427](https://github.com/apache/rocketmq/pull/1427)
precisely identified and proposed `putIfAbsent` for the classic-manager
first-commit race. It did not cover RocksDB v1, incremental whole-map WAL
ordering, or deterministic concurrency/persistence tests, so this issue extends
that valid prior analysis rather than claiming the classic root cause is new.
Current open PRs [#10625](https://github.com/apache/rocketmq/pull/10625),
[#9602](https://github.com/apache/rocketmq/pull/9602), and
[#9877](https://github.com/apache/rocketmq/pull/9877) touch related
manager/test files but do not change these commit paths or solve either race.
This fix will stay limited to classic initialization, RocksDB v1
initialization/incremental ordering, and their regression tests; v2 and
offset-removal behavior are out of scope.
I am working on a focused fix and will submit a PR against `develop`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
