lizhimins opened a new issue, #10827: URL: https://github.com/apache/rocketmq/issues/10827
## 中文版 ### 问题背景 Pop 顺序消费（pop orderly / fifo，含 PopKv 路径）依赖 `attemptId` 实现"同一次接收尝试的重入"：客户端长轮询超时或响应丢失后，会**复用同一 attemptId** 重试；broker 侧 `OrderInfo` 识别到相同 attemptId 时放行重入（`needBlock` 返回 false），重新投出同一批消息且不累加 `offsetConsumedCount`，从而避免队头阻塞和消费次数虚增。 ### 问题描述 `PopConsumerService.popAsync` 入口先抢 group@topic 串行锁（`PopConsumerLockService.tryLock`），**失败即空返回（fail-fast）**。携带原 attemptId 的重试请求同样要走这把锁；Pop Broker 模式（queueId=-1 全队列轮转）下同 group@topic 的所有 pop 请求争抢同一把锁，且锁在异步链 `whenComplete` 才释放，其他请求走慢路径（冷读 / tiered store）时锁持有可达秒级，撞锁窗口并不小。 一旦重试撞锁，完整故障链如下（已逐环经代码验证）： 1. 客户端 A pop 到消息（OrderInfo 登记 attemptId=X，未 ack），响应丢失； 2. A 复用 X 重试，撞 group@topic 锁 → 空返回； 3. 请求带 X 挂进长轮询——撞锁本身不致命，窗口内被唤醒则 `checkBlock(X)` 放行可重入，但该场景缺乏唤醒源：消息 A 没收到、永不 ack（无 ack 唤醒）；锁释放不触发 notify；fifo blocked 时 restCount 不累加（无积压唤醒）； 4. 挂满 pollTime 超时，空响应返回； 5. 空响应对客户端是一次"成功的接收"，客户端轮换出新 attemptId Y（仅 DEADLINE_EXCEEDED 才复用 attemptId）→ `checkBlock(Y)` 与 X 不匹配 → **重入资格永久丢失**。 后果：队列被 X 的未 ack 批次 block，直到 `nextVisibleTime` 到期自愈（卡一个 invisibleTime）；若响应在 proxy→client 之间丢失且 proxy 开启 autoRenew（默认 renew 上限 3h），nextVisibleTime 被持续续期，队列最长卡死约 3h。 实际案例：某巡检系统（gRPC 客户端，收到即 ack）经 Proxy 消费顺序消息，偶发长时间卡住，即由该链路触发。 ### 期望行为 携带相同 attemptId 的顺序消费重试是幂等的只读重入请求，**不应因锁竞争而空返回**——空返回会把重试推进长轮询、消耗唯一的重入机会，最终导致队头阻塞。 ### 修复方案（已实现并验证） 方案讨论中评估过"同 attemptId 锁前只读重放 OrderInfo 上一批结果"（按 offsetList 定点读消息、保留原 popTime 直接返回），功能完整但代码量大；进一步分析确认：响应丢失那次的 receipt handle 作废属正常 at-least-once 语义，既有 re-pop 路径（checkBlock 放行 → update 重登记，同 attemptId 不累加 consumedCount）自身语义自洽。**真正的缺陷只是撞锁导致重试空返回**，因此修复收敛为： `PopConsumerService.popAsync` 拿锁逻辑改为 `tryLockForPop`：fifo 且 attemptId 非空的请求撞锁时**自旋重试直至拿到锁**（`while (!tryLock) Thread.yield()`），其余请求保持 fail-fast。正确性依据： - 锁持有者必然在 pop 完成时释放（`whenComplete` unlock），最坏由锁服务 2 分钟过期清理兜底，自旋不会死等； - attemptId 撞锁本身极罕见，等待通常毫秒级，忙等成本可忽略； - 拿到锁后走既有 re-pop 路径，重入语义完整保留； - wakeUp 长轮询超时重执行链路同样受益。 净改动仅 `PopConsumerService` +26 行，不改 `QueueLevelConsumerManager` / `PopMessageProcessor` / 传统路径，无配置开关。 ### 验证 - 单测 `PopConsumerServiceLockRetryTest`：自旋至拿锁成功进入 pop 路径 / 非 fifo 保持 fail-fast；`PopConsumerServiceTest` 17 例回归全过； - 真实环境（k8s micro 规格，broker 镜像含修复）：端到端场景——首投 FOUND/2 未 ack → 同 attemptId 重 pop FOUND/2（毫秒级）→ ack 成功 → 新 attemptId pop 队列已空；并发撞锁探针——5 轮 × 16 并发同 attemptId pop 共 80 次全部返回同一批消息、零空返回，broker 日志确认 16/16 请求均进锁执行（撞锁窗口真实发生）。 --- ## English Version ### Background Pop orderly consumption (fifo, including the PopKv path) relies on `attemptId` to make one receive attempt reentrant: after a long-polling timeout or a lost response, the client retries with the **same attemptId**; the broker recognizes the same attemptId in `OrderInfo` (`needBlock` returns false), re-delivers the same batch without incrementing `offsetConsumedCount`, and thus avoids queue-head blocking and inflated reconsume times. ### Problem `PopConsumerService.popAsync` first acquires the group@topic serialization lock (`PopConsumerLockService.tryLock`) and **fails fast with an empty response on contention**. A retry carrying the original attemptId must also take this lock; in Pop Broker mode (queueId=-1, all queues rotated) every pop of the same group@topic contends on this single lock, which is only released in the async chain's `whenComplete` and can be held for seconds while another pop walks a slow path (cold read / tiered store). The contention window is non-trivial. Once such a retry collides with the lock, the full failure chain (each step verified against code) is: 1. Client A pops messages (OrderInfo registers attemptId=X, unacked), the response is lost; 2. A retries reusing X, hits the group@topic lock → empty response; 3. The request suspends in long polling with X — the collision alone is not fatal (a wakeup inside the window would let `checkBlock(X)` pass), but no wakeup source exists in this scenario: A never received the messages so no ack wakeup; lock release triggers no notify; restCount is not accumulated while fifo-blocked so no backlog wakeup; 4. The request hangs until pollTime expires and returns an empty response; 5. An empty response is a "successful receive" to the client, which rotates to a new attemptId Y (only DEADLINE_EXCEEDED reuses the attemptId) → `checkBlock(Y)` mismatches X → **the reentrant credential is permanently lost**. Consequence: the queue stays blocked on X's unacked batch until `nextVisibleTime` expires (stuck for one invisibleTime); if the response was lost between proxy and client and proxy autoRenew is on (default renew cap 3h), nextVisibleTime keeps being extended and the queue can be stuck for up to ~3 hours. Real incident: a patrol system (gRPC client, ack-on-receive) consuming orderly messages through Proxy intermittently hung for a long time, traced to exactly this chain. ### Expected Behavior An orderly retry carrying the same attemptId is an idempotent reentrant request and **must not return empty due to lock contention** — an empty response pushes the retry into long polling, burns the only reentrant opportunity, and ends up blocking the queue head. ### Proposed Fix (implemented and verified) A lock-free read-only replay of the previous OrderInfo batch for the same attemptId was evaluated and works, but is much larger; further analysis confirmed that invalidating the receipt handle of the lost delivery is normal at-least-once semantics and the existing re-pop path (checkBlock passes → update re-registers, consumedCount not incremented for the same attemptId) is self-consistent. **The real defect is only that lock contention leaves the retry empty**, so the fix converges to: Change the lock acquisition in `PopConsumerService.popAsync` to `tryLockForPop`: requests with fifo=true and a non-empty attemptId **spin-retry until the lock is acquired** (`while (!tryLock) Thread.yield()`); all other requests keep the fail-fast behavior. Correctness rationale: - The lock holder always releases on pop completion (`whenComplete` unlock), with the lock service's 2-minute expiry sweep as the worst-case backstop, so the spin cannot wait forever; - Same-attemptId contention is rare and the wait is normally milliseconds, so busy-wait cost is negligible; - Once the lock is acquired, the existing re-pop path runs and the reentrant semantics are fully preserved; - The wakeUp long-polling re-execution path benefits as well. Net change is +26 lines in `PopConsumerService` only; no changes to `QueueLevelConsumerManager` / `PopMessageProcessor` / the legacy path, and no configuration switch. ### Verification - Unit tests `PopConsumerServiceLockRetryTest`: spin until lock acquired and proceed into the pop path / non-fifo keeps fail-fast; all 17 existing `PopConsumerServiceTest` cases pass; - Real environment (k8s micro setup, broker image with the fix): end-to-end — first pop FOUND/2 unacked → same-attemptId re-pop FOUND/2 within milliseconds → ack succeeds → fresh-attemptId pop finds the queue drained; concurrent contention probe — 5 rounds × 16 concurrent same-attemptId pops, all 80 returned the same batch with zero empty responses, and broker logs confirmed all 16 requests per round executed inside the lock (contention window genuinely exercised). -- 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]
