fix(ets): carry the sub-token refill remainder across hits

[matthewmathistrellys]

Summary

Hammer.ETS.TokenBucket.hit/5 stamps last_update = now on every allowed hit, but new_tokens is truncated to a whole integer. Any refill smaller than one whole token is therefore discarded, on every hit, permanently.

Two callers are affected:

• A caller hitting faster than one token-period never refills at all. At refill_rate: 55, hits every 5ms each credit trunc(5 * 55 / 1000) == 0 tokens. The bucket drains and only recovers because the deny branch doesn't write.
• A caller paced at its own nominal refill rate slowly starves. At refill_rate: 55 (one token per ~18.18ms), hits every 18ms each credit exactly 1 token but advance the clock a full 18ms, losing ~0.18ms every time. The level decays until the caller is denied at a rate it was entitled to sustain.

The fix

Advance the clock only by the time whose tokens were actually credited, so the remainder carries into the next hit:

last_update + trunc(new_tokens * 1000 / refill_rate)

No storage format change, no new field.

The exception is an overflowing refill. When the bucket fills to capacity the surplus is legitimately discarded, so the clock must snap to now — otherwise a long-idle bucket banks unbounded credit and the next burst is unbounded with it. That applies only when tokens actually accrued. A bucket merely sitting at capacity with new_tokens == 0 has overflowed nothing, and that elapsed time is still owed to the level the hit is about to draw down. Guarding on current_tokens == capacity alone silently throws it away, so the guard is current_tokens == capacity and new_tokens > 0.

On clean/1

Carrying the remainder means the stored timestamp deliberately lags now. Two bounds keep that safe, and both are now pinned by a test:

• it can never run ahead of now, because the credited time is at most the elapsed time (trunc(new_tokens * 1000 / refill_rate) <= elapsed, always);
• it can never lag by more than one token period — at most 1s, since refill_rate >= 1.

clean/1 compares against now - key_older_than, and the documented guidance for that option is hours, so a sub-second lag cannot cause an active row to be reaped.

Tests

Four new tests seed last_update at a known offset and assert the value written back, so they are deterministic rather than throughput measurements — the stored timestamp derives from last_update plus credited time, never from when the test happened to run.

Three of them fail against master:

• carries the sub-token remainder instead of discarding it
• does not reset the clock when a full bucket accrued no new tokens
• a caller paced at the nominal refill rate does not starve — denied at step 11 of 60 on master

The fourth, the stored clock stays between last_update and now, passes on master too. It is a guarantee pin for the bounds above, not regression proof.

Test plan

• mix test — 147 tests, 0 failures (142 on master before these 5 were added)
• Control run: new tests against unmodified master, 3 of 4 fail as described

Notes

Not addressed here, happy to open issues if useful:

• Hammer.Atomic.TokenBucket and both leaky buckets still use System.system_time(:second) and carry the original whole-second bug from ETS TokenBucket: whole-second time granularity caps effective throughput below configured refill_rate when refill_rate > capacity #192. The atomic one is not a drop-in fix — it packs timestamp and fill into one 64-bit word at 32 bits each, and ms-since-epoch needs ~41.
• Both bucket algorithms still return a hardcoded {:deny, 1000} where the other algorithms return a real computed wait. That became computable once the clock moved to milliseconds, but it is a behaviour change, so it seemed worth asking about rather than bundling in here.