messere1 opened a new pull request, #11384: URL: https://github.com/apache/rocketmq/pull/11384
### What is the purpose of the change? Fixes #11383. Six lock acquisition loops in the ack / changeInvisibleTime paths busy-wait with no `Thread.yield()`: - `AckMessageProcessor#ackOrderly` (queue lock manager loop and the consumer-lock-service KV path) - `AckMessageProcessor#ackLite` - `ChangeInvisibleTimeProcessor` (consumer lock service, queue lock manager, and the LMQ lite path) All of these processors run on the shared `ackMessageExecutor`, so each contending request pins one worker thread at 100% CPU for the whole contention window; a burst of same-`(group, topic)` contention can park several workers in bare spins and, on core-saturated brokers, the spinning threads compete for CPU with the very lock holders that must finish for the spins to exit. `PopConsumerService#tryLockForPop` — introduced by #10828 for the same `consumerLockService` — already establishes the pattern: keep the spin (these paths must hold the lock to commit offsets atomically) but `Thread.yield()` in the loop. This change applies that pattern to the remaining bare loops. ### Brief changelog - `broker/src/main/java/org/apache/rocketmq/broker/processor/AckMessageProcessor.java`: add `Thread.yield()` to the three spin loops. - `broker/src/main/java/org/apache/rocketmq/broker/processor/ChangeInvisibleTimeProcessor.java`: add `Thread.yield()` to the three spin loops. No behavioral change: the loops still spin until the lock is acquired (or the entry is self-healed by `PopConsumerLockService.removeTimeout()`); `yield()` is only a scheduler hint so a waiting worker gives its core to the holder. ### Verifying this change - `mvn -pl broker test -Dtest='AckMessageProcessorTest,ChangeInvisibleTimeProcessorTest'` — 18/18 pass on the patched tree. - No new test added, honestly: the change is a scheduler hint inside a tight acquire loop whose observable contract (acquire-or-spin) is identical; the existing processor tests pin the surrounding commit/offset logic. The spin itself is exercised by those tests' orderly-ack paths. - Environment note: the local build stops after tests at `jacoco:0.8.5:report` (JaCoCo 0.8.5 cannot parse Java 17 bytecode) — this reproduces identically on a clean develop checkout and is unrelated to this change (same note as #11380). Follow this checklist to help us contribute code more easily: - [x] Make sure there is a [Github issue](https://github.com/apache/rocketmq/issues/11383) filed for the change (usually before you start working on it). The issue is referenced in the title. - [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why. - [x] Write necessary unit-test to verify your logic correction, when there is a logic change. - [x] Run `mvn -pl broker test -Dtest='AckMessageProcessorTest,ChangeInvisibleTimeProcessorTest'` to make sure unit-test passes. - [x] Update documentation if the change is user-facing. -- 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]
