apache / apache/rocketmq

[Bug] Concurrent first offset commits can overwrite queues in consumer offset managers

Open
#10,700 4 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
22.6k
Forks
12k
Avg merge
3d 1h
Merged PRs (30d)
27

Description

### 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`.

Contributor guide

Open the contributing guide

Research direction

Start with the classic ConsumerOffsetManager and RocksDB v1 manager commit paths, then review the deterministic concurrency test plan in this issue. Verify concurrent first commits retain both queue IDs in memory and after persistence/reload, including the incremental WAL case; RocksDB v2 and offset removal are out of scope.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
databases, distributed-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.