unbridled-41 opened a new pull request, #11048:
URL: https://github.com/apache/rocketmq/pull/11048
Closes #11047
### Problem / Evidence
`TimerEnqueuePutService#fetchAndPutTimerRequest` retries the **whole** batch
when any single request fails:
```java
while (!isStopped()) {
CountDownLatch latch = new CountDownLatch(trs.size());
for (TimerRequest req : trs) { // <-- every request, including
already-succeeded ones
req.setLatch(latch);
... this.putMessageToTimerWheel(req);
}
checkDequeueLatch(latch, -1);
boolean allSuccess = trs.stream().allMatch(TimerRequest::isSucc);
...
}
```
If one request's `doEnqueue` fails (e.g. `TimerLog#append` returns -1 when a
new mapped file cannot be allocated in time at rollover, or any unexpected
throwable while `timerSkipUnknownError=false`), every subsequent round
re-invokes `putMessageToTimerWheel` for the requests whose `doEnqueue` already
succeeded — each round appends another `TimerLog` unit for the same message
into the same timer-wheel slot. On dequeue, every unit is an independent
`MAGIC_DEFAULT` record, so the scheduled message is delivered to the real topic
**once per redundant unit** (and the slot `num` counter is inflated, skewing
`getAllNum`/`isReject` flow-control decisions).
Deterministic regression test
`TimerMessageStoreTest#testEnqueueRetryDoesNotReprocessSucceededRequests`
(counts `doEnqueue` invocations per physical offset; `doEnqueue` is made to
fail exactly once, for the first attempt of one of two batched requests). Fails
on unmodified `develop`:
```
java.lang.AssertionError: expected:<1> but was:<2]
at
org.apache.rocketmq.store.timer.TimerMessageStoreTest.testEnqueueRetryDoesNotReprocessSucceededRequests(TimerMessageStoreTest.java:703)
Tests run: 12, Failures: 1, Errors: 0, Skipped: 0 (develop @ ff8f6f74c +
test only, 2026-09-05)
```
The failed assertion is the already-succeeded request having been enqueued a
second time.
### Root cause / Fix
The retry loop re-processes the whole batch instead of the failed subset.
Fix: filter the batch on `TimerRequest#isSucc()` before each retry round and
only re-put the requests that have not succeeded yet. This is safe for the
dequeue-routed path: requests handed to `dequeuePutQueue` are released (with
`succ=true`) by `TimerDequeuePutService` before the shared latch completes, so
they are never re-put either. Round 1 is unchanged (nothing is successful yet,
the filtered list equals the batch), and the unknown-error hold semantics are
preserved.
### Priority
PRIORITY = 78（影响 32 + 波及范围 16 + 可复现 16 + 维护价值 14），FIX_CONFIDENCE = 85。
- 影响 32/40: duplicate delivery of scheduled/delay messages — a user-visible
message-correctness violation on a realistic transient failure (IO pressure at
TimerLog rollover, unexpected errors without the skip flag).
- 波及范围 16/20: the timer wheel backs all delayed/scheduled messages; every
partial-batch enqueue failure duplicates up to the whole rest of the batch
(batches are up to ~10 requests).
- 可复现 16/20: fully deterministic unit test by injecting a one-shot
`doEnqueue` failure; production trigger requires a partial batch failure but
the retry-duplication itself is certain.
- 维护价值 14/20: small, local fix to the retry loop; clearly separable from the
surrounding logic.
### Tests
- Regression test added:
`TimerMessageStoreTest#testEnqueueRetryDoesNotReprocessSucceededRequests`
(fails before the fix as shown above).
- After the fix (`develop @ ff8f6f74c` + this change, 2026-09-05):
```
mvn -pl store test -Dtest=TimerMessageStoreTest
Tests run: 12, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS
mvn -pl store test
-Dtest='TimerCheckPointTest,TimerLogTest,TimerWheelTest,TimerMetricsTest,TimerEngineSwitchVerifyTest'
Tests run: 21, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS
```
### Risk
Low. Round 1 behavior is identical (no request is successful before the
first round, so the filtered retry list equals the original batch). Requests
that succeed are simply excluded from later rounds, which is exactly the
invariant the commit offset advance (`commitQueueOffset =
trs.get(trs.size()-1)...`) already assumes — the batch is only acknowledged
once every request succeeded. The `holdMomentForUnknownError` backoff and the
`timerWheelSnapshotFlush` locking are unchanged.
--
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]
