apache / apache/rocketmq

[Bug] Timer enqueue retry re-enqueues already-succeeded requests, delivering scheduled messages multiple times

Open
#11,047 3 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
22.6k
Forks
12k
Avg merge
3d 1h
Merged PRs (30d)
27

Description

### Before Creating the Bug Report

- [x] I found a bug, not just a question.
- [x] I searched open GitHub Issues and pull requests and found no duplicate.
- [x] I confirmed that this bug belongs to Apache RocketMQ.

### Runtime platform environment

All platforms; reproduced with a deterministic unit test on the current `develop` branch.

### RocketMQ version

`develop` at `ff8f6f74c`

### JDK Version

All

### Describe the Bug

When a batch of timer requests is enqueued, `TimerEnqueuePutService#fetchAndPutTimerRequest` retries the **whole** batch after any single request fails. The retry loop re-invokes `putMessageToTimerWheel` for every request in `trs` without checking `TimerRequest#isSucc`, so requests whose `doEnqueue` already succeeded in an earlier round are enqueued **again** — each retry 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 same scheduled message is converted and delivered to the real topic multiple times (and the slot `num` counter is inflated, skewing `getAllNum`/`isReject` flow-control decisions).

A partial batch failure is realistic: `TimerLog#append` returns -1 when a new mapped file cannot be allocated in time (IO pressure at file rollover), and any unexpected throwable while `timerSkipUnknownError=false` fails the current request only — the other requests of the batch have already succeeded.

### Steps to Reproduce

1. Put two timer requests A and B into `enqueuePutQueue` (same or different delay times).
2. Make `doEnqueue` fail exactly once — for A's first attempt only (equivalent to a transient `TimerLog#append` failure).
3. Drive `TimerEnqueuePutService#fetchAndPutTimerRequest` once.
4. Observe that A's `doEnqueue` runs twice: the successful first attempt is re-processed by the retry round, appending a second `TimerLog` unit for the same message.

### What Did You Expect to See?

The retry round must only re-process the requests that did not succeed; already-succeeded requests are never re-enqueued, so the message is delivered exactly once.

### What Did You See Instead?

Round 2 re-enqueues A and the scheduled message is delivered twice (once per appended `TimerLog` unit).

### Additional Context

Proposed fix: filter the batch on `TimerRequest#isSucc()` before each retry round (requests routed to the dequeue path are released with `succ=true` before the shared latch completes, so they are never re-put either). A regression test in `TimerMessageStoreTest` counts `doEnqueue` invocations per physical offset and fails on unmodified `develop`.

### Corresponding PR

- PR apache/rocketmq#11048

Contributor guide

Open the contributing guide

Research direction

Start in TimerEnqueuePutService#fetchAndPutTimerRequest and inspect TimerRequest#isSucc() around the retry loop. Run the regression test in TimerMessageStoreTest, which counts doEnqueue invocations per physical offset. Done means a request that succeeds before a retry is not enqueued again and each scheduled message is delivered once.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
distributed-systems
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.