messere1 opened a new issue, #11379:
URL: https://github.com/apache/rocketmq/issues/11379
### Background
`MessageStoreDispatcherImpl#doScheduleDispatch` is the tiered-storage
dispatch loop. Before entering the group-commit append loop it preflights the
*first* offset and guards both lookups (added by #8965 / issue #8955, which
fixed the dispatch thread exiting on NPE):
```java
CqUnit cqUnit = consumeQueue.get(currentOffset);
if (cqUnit == null) { log.warn(...); return ...completedFuture(false); }
SelectMappedBufferResult message =
defaultStore.selectOneMessageByOffset(cqUnit.getPos(), cqUnit.getSize());
if (message == null) { log.warn(...); return ...completedFuture(false); }
```
However, the append loop that follows re-fetches the CQ unit and the message
for every subsequent offset **without any of those guards**:
```java
for (; offset < targetOffset; offset++) {
cqUnit = consumeQueue.get(offset);
bufferSize += cqUnit.getSize(); // NPE if cqUnit
== null
...
message = defaultStore.selectOneMessageByOffset(cqUnit.getPos(),
cqUnit.getSize());
appendingBufferList.add(message);
AppendResult result = flatFile.appendCommitLog(message); // NPE/garbage
if message == null
...
}
```
### Problem
1. **Mid-loop NPE.** `ConsumeQueueInterface#get` returns null once the
consume queue has been truncated past the requested offset, and
`selectOneMessageByOffset` returns null once the commitlog data has been
deleted (expired files). Both events happen through the ordinary expiry sweeps
while the dispatcher is catching up (e.g. after a downtime longer than the file
retention). The preflight only proves the *first* offset of the batch is still
there; the batch spans up to `tieredStoreGroupCommitCount` more offsets, so the
lookups can fail inside the loop. The resulting NPE escapes
`doScheduleDispatch` (only `ConsumeQueueException` is caught there).
2. **Message buffer leak.** Every `SelectMappedBufferResult` fetched inside
the loop is added to `appendingBufferList` and is only ever released through
`GroupCommitContext#release()`. When the loop dies with the NPE above, the
context is never constructed, so every buffer fetched before the failing offset
leaks its `MappedFile` reference.
The same leak exists on a non-exceptional path: if `appendCommitLog`
returns `SUCCESS` but the following `appendConsumeQueue` fails on the first
message, the loop breaks with `appendingBufferList` non-empty and
`dispatchRequestList` empty. The context is built, but the `if
(!dispatchRequestList.isEmpty())` block — the only place that hands it to
`commitAsync`/`constructIndexFile`, both of which release it — never runs. The
context is silently dropped and its buffers leak.
### Impact
- Repeated mapped-buffer/reference leaks on queues whose data expires while
dispatch lags, and on flat files that fail a consume-queue append after a
successful commitlog append — eventually holding expired commitlog files from
being cleaned.
- The NPE aborts the current dispatch round for the queue (the dispatcher
thread itself survives because `dispatch()` catches `Throwable`), so dispatch
progress depends on the next cycle re-failing the preflight instead of
degrading gracefully.
### Suggestion
Mirror the preflight guards inside the loop (`break` on a missing CQ unit or
message so the partial batch still commits and the context is still released),
and release the context when nothing was appended to the consume queue so no
code path will consume it.
--
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]
