Skip to content

fix(backends): retry Memcached increment when a new counter expires before INCR - #318

Merged
allen0099 merged 1 commit into
masterfrom
fix/memcached-increment-expiry-race
Sep 27, 2026
Merged

allen0099 merged 1 commit into
masterfrom
fix/memcached-increment-expiry-race

Conversation

@allen0099

Copy link
Copy Markdown
Owner

Problem

MemcachedBackend.increment() creates a counter with INCR (miss), ADD with the exptime, then INCR again. Memcached keeps time in whole seconds, so an item stored with exptime 1 can expire almost immediately if the clock ticks right after the ADD. When that happened between the ADD and the second INCR, increment() raised CacheXError("Counter vanished between ADD and INCR"). This failed once in CI in test_memcached_increment_honors_ttl.

Fix

  • _increment now retries the ADD + INCR pair up to _CAS_MAX_RETRIES (16) times, the same bound as the CAS loops, and returns as soon as an INCR lands. A counter that expired inside its own window really is a new window, so starting again at delta is correct. The whole loop still runs inside the one asyncio.to_thread call.
  • Only after every attempt misses does increment() raise CacheXError("Counter vanished between ADD and INCR on each of 16 attempts").
  • Redis and memory have no analogous race: Redis does INCRBY + EXPIRE in one Lua script (atomic, and keys cannot expire mid-script), memory checks and creates under its lock with a single now.

Tests

  • New stubbed test: the INCR after the first ADD misses and the retry succeeds.
  • The existing "vanished" test now checks the bound: 16 ADDs, 17 INCRs, then CacheXError.
  • test_memcached_increment_honors_ttl uses ttl=2 (the counter survives the next call regardless of the tick) and polls for expiry with a 5 s deadline instead of a fixed sleep. It also asserts the second call returns 2, so the window was really still open. Ran it 10 times in a row: all passed (1.3 s to 2.1 s each).

Full suite including the live Redis and Memcached tests: 1236 passed, 1 skipped, coverage 99.97%. ruff, ruff format and mypy --strict are clean.

Docs

docs/BACKENDS.md and the zh-TW mirror describe the retry under increment. Changelog fragment: changelog.d/315.fixed.md.

Closes #315

…efore INCR

Memcached keeps time in whole seconds, so a counter created with ttl=1
can expire between its ADD and the INCR that follows, and increment()
raised 'Counter vanished between ADD and INCR'. Retry the ADD + INCR
pair up to _CAS_MAX_RETRIES times inside the same worker call; a counter
that expired inside its own window starts a new one at delta.

The live ttl test now uses ttl=2 and polls for expiry with a deadline
instead of depending on where the clock tick lands.

Closes #315
@allen0099 allen0099 added this to the 0.3.9 milestone Sep 27, 2026
@allen0099
allen0099 merged commit cff36a4 into master Sep 27, 2026
12 checks passed
@allen0099
allen0099 deleted the fix/memcached-increment-expiry-race branch September 27, 2026 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memcached increment(ttl=1) can raise 'Counter vanished between ADD and INCR' on a fresh key

1 participant