Before Creating the Bug Report
Runtime platform environment
Reproduced on Ubuntu 22.04.4 LTS, Linux 5.15.0-186, x86_64. The defect is in the broker's asynchronous POP revive control flow and is not expected to be OS-specific.
RocketMQ version
- Branch:
develop
- Version: current
5.5.1 development sources
- Commit:
e348efa66b08eb645ee123706ea6492fa9a3ad35
JDK Version
OpenJDK 8u502 (Maven runtime).
Describe the Bug
Proposed severity: High, because the failure can violate POP's at-least-once redelivery guarantee for an unacknowledged message.
PopReviveService.reviveMsgFromCk starts an asynchronous business-message read for every unacknowledged offset and records the checkpoint in inflightReviveRequestMap. mergeAndRevive then advances and commits the revive offset after scheduling that work.
If EscapeBridge.getMessageAsync (including its decode/transform chain) completes exceptionally, the corresponding child future remains exceptional. CompletableFuture.allOf(...).whenComplete(...) still invokes its callback, but the callback calls future.getNow(...); for an exceptionally completed future, getNow throws CompletionException. The callback therefore exits before it can:
- call
rePutCK to preserve retryability;
- mark the in-flight checkpoint complete;
- remove it from
inflightReviveRequestMap.
The only timeout fallback is inside while (inflightReviveRequestMap.size() > 3). With one to three failed checkpoints and no later traffic, that branch is never entered. Because the revive offset has already moved past the original checkpoint, the unacknowledged message can remain unredelivered indefinitely.
This is not a security issue.
Steps to Reproduce
A deterministic broker-free unit reproduction is sufficient:
- Build one
PopCheckPoint containing a single unacknowledged message.
- Make
EscapeBridge.getMessageAsync(...) return an exceptionally completed CompletableFuture.
- Invoke
PopReviveService.mergeAndRevive(...).
- Assert that the revive offset and committed offset advanced, the checkpoint remains in the in-flight map, and no replacement CK was written.
The core setup is:
CompletableFuture<Triple<MessageExt, String, Boolean>> failed = new CompletableFuture<>();
failed.completeExceptionally(new RuntimeException("store read failed"));
when(escapeBridge.getMessageAsync(anyString(), anyLong(), anyInt(), anyString(), anyBoolean()))
.thenReturn(failed);
popReviveService.mergeAndRevive(reviveObj);On the unmodified baseline, two independent runs reproduced the same state:
reviveObj.newOffset = 1
committed revive offset = 1
inflightReviveRequestMap.size() = 1
messageStore.putMessage(rewritten CK) = 0 calls
The regression test consequently fails because one CK rewrite was expected but none occurred.
What Did You Expect to See?
An exceptional business-message read should be treated as a retryable read failure. The service should rewrite the checkpoint for that message offset, finish/remove the in-flight record, and preserve the existing offset progression without losing the message's retry path.
What Did You See Instead?
The original revive offset is committed, but the replacement CK is never written. At low traffic the incomplete in-flight record is not old enough to trigger any independent scan and the size-gated timeout loop is never entered, so the message can be skipped indefinitely.
Additional Context
A minimal fix is to use CompletableFuture.handle on the getBizMessage stage. If the upstream read completes exceptionally, log it and return (msgOffset, false), which feeds the existing rePutCK path. The handler should remain scoped to the read stage so exceptions raised later while processing/writing a retry retain their current semantics.
With that fix, the full PopReviveServiceTest class passes 13/13 tests. Checkstyle, SpotBugs, and git diff --check also pass.
Exact GitHub searches for PopReviveService getNow, PopReviveService rePutCK exception, getBizMessage PopReviveService exception, and PopReviveService CompletionException found no issue or PR match. Related #10658/#10659 concern head-of-line blocking in the newer PopConsumerService popkv batch path. #10667 concerns discarded futures in PopConsumerCache.cleanupRecords when buffer merging is enabled. Neither covers the legacy/default PopReviveService path committing an offset without preserving retry after an exceptional read.
Before Creating the Bug Report
apache/rocketmqrepository.Runtime platform environment
Reproduced on Ubuntu 22.04.4 LTS, Linux 5.15.0-186, x86_64. The defect is in the broker's asynchronous POP revive control flow and is not expected to be OS-specific.
RocketMQ version
develop5.5.1development sourcese348efa66b08eb645ee123706ea6492fa9a3ad35JDK Version
OpenJDK 8u502 (Maven runtime).
Describe the Bug
Proposed severity: High, because the failure can violate POP's at-least-once redelivery guarantee for an unacknowledged message.
PopReviveService.reviveMsgFromCkstarts an asynchronous business-message read for every unacknowledged offset and records the checkpoint ininflightReviveRequestMap.mergeAndRevivethen advances and commits the revive offset after scheduling that work.If
EscapeBridge.getMessageAsync(including its decode/transform chain) completes exceptionally, the corresponding child future remains exceptional.CompletableFuture.allOf(...).whenComplete(...)still invokes its callback, but the callback callsfuture.getNow(...); for an exceptionally completed future,getNowthrowsCompletionException. The callback therefore exits before it can:rePutCKto preserve retryability;inflightReviveRequestMap.The only timeout fallback is inside
while (inflightReviveRequestMap.size() > 3). With one to three failed checkpoints and no later traffic, that branch is never entered. Because the revive offset has already moved past the original checkpoint, the unacknowledged message can remain unredelivered indefinitely.This is not a security issue.
Steps to Reproduce
A deterministic broker-free unit reproduction is sufficient:
PopCheckPointcontaining a single unacknowledged message.EscapeBridge.getMessageAsync(...)return an exceptionally completedCompletableFuture.PopReviveService.mergeAndRevive(...).The core setup is:
On the unmodified baseline, two independent runs reproduced the same state:
The regression test consequently fails because one CK rewrite was expected but none occurred.
What Did You Expect to See?
An exceptional business-message read should be treated as a retryable read failure. The service should rewrite the checkpoint for that message offset, finish/remove the in-flight record, and preserve the existing offset progression without losing the message's retry path.
What Did You See Instead?
The original revive offset is committed, but the replacement CK is never written. At low traffic the incomplete in-flight record is not old enough to trigger any independent scan and the size-gated timeout loop is never entered, so the message can be skipped indefinitely.
Additional Context
A minimal fix is to use
CompletableFuture.handleon thegetBizMessagestage. If the upstream read completes exceptionally, log it and return(msgOffset, false), which feeds the existingrePutCKpath. The handler should remain scoped to the read stage so exceptions raised later while processing/writing a retry retain their current semantics.With that fix, the full
PopReviveServiceTestclass passes 13/13 tests. Checkstyle, SpotBugs, andgit diff --checkalso pass.Exact GitHub searches for
PopReviveService getNow,PopReviveService rePutCK exception,getBizMessage PopReviveService exception, andPopReviveService CompletionExceptionfound no issue or PR match. Related #10658/#10659 concern head-of-line blocking in the newerPopConsumerServicepopkv batch path. #10667 concerns discarded futures inPopConsumerCache.cleanupRecordswhen buffer merging is enabled. Neither covers the legacy/defaultPopReviveServicepath committing an offset without preserving retry after an exceptional read.