Bug: TimingWheel.MoveTimer can fire one revolution early or late
Description
TimingWheel.MoveTimer (and SetTimer on an existing key) can fire a task a full revolution early or late when the move is issued after the wheel has been running — i.e. whenever the task's old slot differs from the current pointer position.
Environment
- OS: macOS 26.7 (arm64) — the reproduction below drives time manually via
FakeTicker, so it is platform/time-independent - Go: go1.27.1
- go-zero: master @ 84c92d71
Minimal reproduction
func TestMoveMidFlight(t *testing.T) {
// 6-slot wheel; SetTimer 20s at t=0, MoveTimer 14s right after tick 3
ticker := timex.NewFakeTicker()
var tickNow, fireTick int32 = 0, -1
tw, _ := collection.NewTimingWheelWithTicker(time.Second, 6, func(k, v any) {
atomic.StoreInt32(&fireTick, atomic.LoadInt32(&tickNow))
}, ticker)
defer tw.Stop()
tw.SetTimer("A", 1, 20*time.Second)
for i := 1; i <= 30; i++ {
atomic.StoreInt32(&tickNow, int32(i))
ticker.Tick()
time.Sleep(3 * time.Millisecond)
if i == 3 {
tw.MoveTimer("A", 14*time.Second)
time.Sleep(3 * time.Millisecond)
}
}
t.Log("fired at:", atomic.LoadInt32(&fireTick))
}Intended fire: t=17 (move at t=3 + 14s). Actual: t=23, six ticks (one revolution) late. The mirror geometry (task slot ahead of the pointer, multi-revolution move) fires one revolution early: Set 18s + Move 23s at t=4 on a 12-slot wheel → intended t=27, actual t=15.
Root cause
moveTask derives pos/circle from getPositionAndCircle, whose (steps-1)/N circle formula is written for a fresh entry hung directly at the target slot. But a moved task physically stays in its old slot Q; the circle countdown and the diff hop only take effect when the pointer scans Q, which is
a = ((Q - tickedPos - 1) mod N) + 1 // 1..N ticks awayThis arc a is never credited against the delay budget, so the actual fire time is a + circle*N + diff while the intended one is steps:
fire - steps = (Q-P-1 mod N) + pos - Q - ((steps-1) mod N) ≡ 0 (mod N)The written diff is always correct (≡ steps - a, mod N), so the entire error lands on circle: it is exactly 0 or ±N.
- old slot already passed this revolution, target ahead (
pos >= timer.posbranch): fires N late - old slot not yet passed, target behind & multi-revolution (
pos < timer.pos && circle > 0branch): fires N early
Why existing tests don't catch it
All current Move tests issue SetTimer and MoveTimer at the same instant (same anchor position), a geometry where the arithmetic happens to be exact.
Impact
core/collection/cache.go calls MoveTimer on every Set of an existing key (wheel: 1s x 300 slots, one revolution = 5 minutes), so in-memory cache expiry can be off by up to ±5 minutes. Tasks are never lost or duplicated, only mistimed. stores/cache/cleaner.go is unaffected (it re-SetTimers a fresh key after the previous entry fired).
Suggested fix
Credit the arc a before converting the remaining budget:
steps := int(task.delay / tw.interval)
// ticks until the pointer next scans the old slot, in [1, numSlots]
var a int
if tw.tickedPos > timer.pos {
a = tw.numSlots - (tw.tickedPos - timer.pos) // old slot behind: wrap around
} else if tw.tickedPos == timer.pos {
a = tw.numSlots // current slot was just scanned: wait a full revolution
} else {
a = timer.pos - tw.tickedPos // old slot ahead: walk the difference
}
// the three branches above are equivalent to the one-liner:
// a = (timer.pos-tw.tickedPos-1+tw.numSlots)%tw.numSlots + 1
if steps < a {
// intended fire precedes the next scan of the old slot: re-hang immediately
timer.item.removed = true
newItem := &timingEntry{baseEntry: task, value: timer.item.value}
pos, circle := tw.getPositionAndCircle(task.delay)
newItem.circle = circle
tw.slots[pos].PushBack(newItem)
tw.setTimerPosition(pos, newItem)
return
}
timer.item.circle = (steps - a) / tw.numSlots
timer.item.diff = (steps - a) % tw.numSlotsAll existing tests pass unchanged with this fix; I also have table-driven regression tests for mid-flight moves (the geometry above) ready to PR if welcome.
Source: zeromicro/go-zero