Copilot commented on code in PR #10567:
URL: https://github.com/apache/rocketmq/pull/10567#discussion_r3503200647
##########
broker/src/main/java/org/apache/rocketmq/broker/processor/PopBufferMergeService.java:
##########
@@ -445,6 +445,11 @@ private boolean checkQueueOk(PopCheckPointWrapper
pointWrapper) {
*/
public boolean addCkJustOffset(PopCheckPoint point, int reviveQueueId,
long reviveQueueOffset,
long nextBeginOffset) {
+ if (this.counter.get() >
brokerController.getBrokerConfig().getPopCkMaxBufferSize()) {
+ POP_LOGGER.warn("[PopBuffer]add ck just offset, max size, {}, {}",
point, this.counter.get());
+ return false;
+ }
Review Comment:
This change introduces new backpressure behavior in `addCkJustOffset()` when
the buffer reaches `popCkMaxBufferSize`, but there is no test asserting the
max-size path. Please add/extend a unit test (e.g., in
`PopBufferMergeServiceTest`) that sets a small `getPopCkMaxBufferSize()` and
verifies `addCkJustOffset()` returns false once the limit is hit.
##########
broker/src/main/java/org/apache/rocketmq/broker/processor/PopBufferMergeService.java:
##########
@@ -445,6 +445,11 @@ private boolean checkQueueOk(PopCheckPointWrapper
pointWrapper) {
*/
public boolean addCkJustOffset(PopCheckPoint point, int reviveQueueId,
long reviveQueueOffset,
long nextBeginOffset) {
+ if (this.counter.get() >
brokerController.getBrokerConfig().getPopCkMaxBufferSize()) {
+ POP_LOGGER.warn("[PopBuffer]add ck just offset, max size, {}, {}",
point, this.counter.get());
+ return false;
+ }
+
Review Comment:
The max-size guard can still allow the buffer to exceed
`popCkMaxBufferSize`: when `counter == max`, this check passes and the later
`incrementAndGet()` will push the count to `max + 1` (and under concurrency it
can overshoot by more). For an actual capacity cap/backpressure, reserve
capacity with a CAS loop (or similar) and remove the separate increment at the
end; also make sure to decrement on early-return paths like mergeKey conflict.
--
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]
