messere1 opened a new issue, #11383:
URL: https://github.com/apache/rocketmq/issues/11383
### Before Creating the Bug Report
- [X] I found a bug. The issue list contains no issue matching my bug.
### Your Environment
- RocketMQ version: 5.4.x (current develop)
### Describe the Bug
Several pop/ack paths acquire the consumer/queue lock with a bare busy-wait
that has no `Thread.yield()`:
- `AckMessageProcessor#ackOrderly` — `while
(!queueLockManager.tryLock(lockKey)) {}`
(broker/src/main/java/org/apache/rocketmq/broker/processor/AckMessageProcessor.java:405)
- `AckMessageProcessor#ackOrderly` (KV service path) — `while
(!consumerLockService.tryLock(consumeGroup, topic)) {}`
(AckMessageProcessor.java:448)
- `AckMessageProcessor#ackLite` — `while
(!consumerLockService.tryLock(lockKey)) {}` (AckMessageProcessor.java:538)
- `ChangeInvisibleTimeProcessor#changeInvisibilityDuration` — same shape
(ChangeInvisibleTimeProcessor.java:203, :235, :389)
All of these are registered on the shared `ackMessageExecutor`
(BrokerController.java:1204-1213), so each contending request pins one worker
thread at 100% CPU for the entire contention window:
1. Under a `(group, topic)` contention burst, several executor workers can
be stuck spinning at once, cutting the executor's effective parallelism for
every other ack / changeInvisibleTime request on the broker.
2. On core-saturated brokers the spinning threads compete for CPU with the
very lock holders that must finish for the spin to exit, extending the wait
beyond the actual critical-section length.
The codebase already establishes the correct pattern for this lock service:
`PopConsumerService#tryLockForPop` (added by #10828) spins with
`Thread.yield()` and a comment explaining why the spin is kept — same
`consumerLockService`, same contention shape, but it yields.
### Steps to Reproduce
Code inspection + any workload where two consumers of the same group
ack/changeInvisibleTime concurrently for the same topic: the losing request's
thread spins in the `while` loop with no yield until the holder releases (or
`PopConsumerLockService.removeTimeout()` self-heals a stale entry).
### What Did You Expect to See?
The spin semantics preserved (these paths must hold the lock to commit
offsets atomically) but with `Thread.yield()` in the loop, matching
`tryLockForPop` — so a contending worker yields its core to the holder instead
of burning it.
### Additional Context
None
--
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]
