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

changelog.d/315.fixed.md

@@ -0,0 +1,7 @@
+**`MemcachedBackend.increment()` no longer fails when a new short-lived counter expires mid-call.**
+Creating a counter takes an `ADD` and then an `INCR`, and Memcached keeps time
+in whole seconds, so a counter created with `ttl=1` could expire between the
+two and `increment()` raised `CacheXError("Counter vanished between ADD and
+INCR")`. The `ADD` + `INCR` pair is now retried up to 16 times in the same
+worker call, starting a new window at `delta`; `CacheXError` is raised only if
+the counter vanishes on every attempt.

docs/BACKENDS.md

@@ -243,8 +243,11 @@ if await backend.set_if_absent(f"stream:{user_id}", owner, ttl=300):
 
 - `increment(key, delta=1, ttl=None) -> int` — Memory does the read-modify-write
   under its lock, Redis runs a Lua script (`EXISTS` + `INCRBY` + `EXPIRE`) and
-  Memcached uses `ADD` + `INCR`/`DECR` (Memcached counters stop at 0). The
-  counter is visible through `get()` as a `CacheEntry` with fingerprint
+  Memcached uses `ADD` + `INCR`/`DECR` (Memcached counters stop at 0).
+  Memcached keeps time in whole seconds, so a new counter with a short `ttl`
+  can expire between its `ADD` and the `INCR`; Memcached then retries `ADD` +
+  `INCR`, starting a new window at `delta`, and raises `CacheXError` only if
+  the counter vanishes on all 16 attempts. The counter is visible through `get()` as a `CacheEntry` with fingerprint
   `COUNTER_FINGERPRINT` and the decimal value as content, so `delete`/`clear*`
   and the monitoring routes treat it like any other entry. Incrementing a key
   that holds anything else raises `CacheXError` on every backend, even a cached

fastapi_cachex/backends/memcached.py

@@ -316,20 +316,42 @@ def _add_delta(self, prefixed_key: str, delta: int) -> int | None:
         return None if result is None else int(result)
 
     def _increment(self, prefixed_key: str, delta: int, exptime: int) -> int | None:
-        """Run ``increment``'s INCR, ADD and retried INCR in one worker thread."""
+        """Run ``increment``'s INCR, ADD and retried INCR in one worker thread.
+
+        Returns ``None`` only when the counter vanished after every one of
+        ``_CAS_MAX_RETRIES`` ADD + INCR attempts.
+        """
         value = self._add_delta(prefixed_key, delta)
-        if value is None:
+        if value is not None:
+            return value
+        for _ in range(_CAS_MAX_RETRIES):
             # No counter yet: ADD is atomic and a no-op when a concurrent
-            # call created it first, so the retry always finds a counter.
+            # call created it first.
             self.client.add(prefixed_key, b"0", exptime, noreply=False)
             value = self._add_delta(prefixed_key, delta)
-        return value
+            if value is not None:
+                return value
+            # Memcached keeps time in whole seconds, so a counter created with
+            # a short ttl can expire before the INCR that follows its ADD. It
+            # expired inside its own window, so the next window starts again
+            # at ``delta``.
+            logger.debug("Memcached INCREMENT RETRY; key=%s", prefixed_key)
+        return None
 
     async def increment(self, key: str, delta: int = 1, ttl: int | None = None) -> int:
         """Atomically add ``delta`` to the counter at ``key`` (see base class).
 
         Memcached counters are unsigned, so a negative ``delta`` uses DECR,
         which stops at 0 instead of going negative.
+
+        Creating a counter takes an ADD and then an INCR. If the new counter
+        expires in between (Memcached's clock has one-second resolution, so a
+        ``ttl=1`` counter can live for well under a second), the ADD + INCR
+        pair is retried, starting a new window at ``delta``.
+
+        Raises:
+            CacheXError: If the key holds a value that is not a counter, or
+                the counter vanished after every ADD + INCR attempt.
         """
         validate_delta(delta)
         validate_ttl(ttl)
@@ -348,7 +370,7 @@ async def increment(self, key: str, delta: int = 1, ttl: int | None = None) -> i
             msg = "Cache key holds a value that is not a counter"
             raise CacheXError(msg) from e
         if value is None:
-            msg = "Counter vanished between ADD and INCR"
+            msg = f"Counter vanished between ADD and INCR on each of {_CAS_MAX_RETRIES} attempts"
             raise CacheXError(msg)
         logger.debug("Memcached INCREMENT; key=%s value=%s ttl=%s", key, value, ttl)
         return value

i18n/zh-TW/docs/BACKENDS.md

@@ -176,7 +176,7 @@ if await backend.set_if_absent(f"stream:{user_id}", owner, ttl=300):
         await backend.delete_if_equals(f"stream:{user_id}", owner)
 ```
 
-- `increment(key, delta=1, ttl=None) -> int`：記憶體後端在鎖內執行讀取—修改—寫入，Redis 執行 Lua 腳本（`EXISTS` + `INCRBY` + `EXPIRE`），Memcached 則使用 `ADD` + `INCR`/`DECR`（Memcached 的計數器最低停在 0）。計數器可透過 `get()` 讀到，形式為 fingerprint 為 `COUNTER_FINGERPRINT`、內容為十進位數值的 `CacheEntry`，因此 `delete`／`clear*` 與監控路由都會把它當成一般項目處理。對存放其他內容的鍵執行 increment，在每個後端上都會拋出 `CacheXError`，即使是本文剛好是數字的快取回應也一樣。以 `set(key, counter_entry(n))` 寫入的計數器在每個後端上都可以 increment，唯一的例外是 Memcached 的計數器沒有正負號：在它上面 `n` 必須介於 0 到 2**64 - 1 之間，對負數的計數器執行 increment 會拋出 `CacheXError`。`delta` 必須是 signed 64 位元範圍內的 `int`，否則會在存取後端之前拋出 `TypeError` 或 `ValueError`。
+- `increment(key, delta=1, ttl=None) -> int`：記憶體後端在鎖內執行讀取—修改—寫入，Redis 執行 Lua 腳本（`EXISTS` + `INCRBY` + `EXPIRE`），Memcached 則使用 `ADD` + `INCR`/`DECR`（Memcached 的計數器最低停在 0）。Memcached 以整秒計時，因此 `ttl` 很短的新計數器可能在 `ADD` 與 `INCR` 之間就過期；此時 Memcached 會重試 `ADD` + `INCR`，從 `delta` 開始新的時間窗，只有連續 16 次嘗試計數器都消失時才拋出 `CacheXError`。計數器可透過 `get()` 讀到，形式為 fingerprint 為 `COUNTER_FINGERPRINT`、內容為十進位數值的 `CacheEntry`，因此 `delete`／`clear*` 與監控路由都會把它當成一般項目處理。對存放其他內容的鍵執行 increment，在每個後端上都會拋出 `CacheXError`，即使是本文剛好是數字的快取回應也一樣。以 `set(key, counter_entry(n))` 寫入的計數器在每個後端上都可以 increment，唯一的例外是 Memcached 的計數器沒有正負號：在它上面 `n` 必須介於 0 到 2**64 - 1 之間，對負數的計數器執行 increment 會拋出 `CacheXError`。`delta` 必須是 signed 64 位元範圍內的 `int`，否則會在存取後端之前拋出 `TypeError` 或 `ValueError`。
 - `get_and_delete(key) -> CacheEntry | None`：記憶體後端在鎖內 pop，Redis 使用 `GETDEL`（伺服器 6.2 以上），Memcached 使用 `GETS` + `exptime=-1` 的 `CAS` 寫入（若中間有其他寫入者替換了值則會重試；連續 16 次都被替換時會拋出 `CacheXError`，而不是當成鍵不存在）。`StateManager.consume_state`、`StateManager.delete_state`、`CacheManager.delete` 與 `invalidate()` 都建立在它之上。
 - `set_if_absent(key, value, ttl=None) -> bool`：只在 `key` 不存在時儲存 `value`（已過期的鍵視為不存在），並回報是否有寫入。記憶體後端在鎖內檢查，Redis 使用 `SET NX EX`，Memcached 使用 `ADD`。
 - `delete_if_equals(key, expected) -> bool`：只在 `key` 仍存放 `expected` 時才移除它，因此項目已過期的持有者無法釋放已被他人取得的鎖。請在你儲存的項目中放入唯一的權杖，並以同一個項目釋放。記憶體後端在鎖內比較，Redis 透過 Lua 腳本刪除，並在腳本中重新檢查先前比較過的值，Memcached 則使用 `GETS` + 一個讓項目立即過期的 `CAS` 寫入（傳統協定的 `DELETE` 不接受 CAS 權杖）。

tests/backends/test_memcached.py

@@ -438,12 +438,23 @@ async def test_memcached_increment_decrement_stops_at_zero(
 async def test_memcached_increment_honors_ttl(
     memcached_backend: MemcachedBackend,
 ) -> None:
-    await memcached_backend.increment("window", ttl=1)
-    await memcached_backend.increment("window", ttl=3600)
-    await asyncio.sleep(2)
+    """The ttl of the first call sets the window; later ttls do not extend it.
 
-    assert await memcached_backend.get("window") is None
-    assert await memcached_backend.increment("window", ttl=1) == 1
+    Memcached keeps time in whole seconds, so an item stored with exptime N
+    lives somewhere between N - 1 and N + 1 seconds. ttl=2 keeps the counter
+    alive across the next call, and polling with a deadline waits for expiry
+    without depending on where the clock tick lands.
+    """
+    assert await memcached_backend.increment("window", ttl=2) == 1
+    assert await memcached_backend.increment("window", ttl=3600) == 2
+
+    loop = asyncio.get_running_loop()
+    deadline = loop.time() + 5
+    while await memcached_backend.get("window") is not None:
+        assert loop.time() < deadline, "counter outlived its 2 s window"
+        await asyncio.sleep(0.2)
+
+    assert await memcached_backend.increment("window", ttl=2) == 1
 
 
 async def test_memcached_increment_only_maps_non_numeric_errors_to_not_a_counter() -> (
@@ -722,17 +733,42 @@ async def spaced() -> dict[str, str]:
 async def test_increment_reports_a_counter_that_vanished_mid_call() -> None:
     """ADD then INCR is two round-trips; the entry can expire in between.
 
-    Memcached has no way to make the pair atomic, so `increment` has to
-    surface the loss instead of returning `None` as if it were a count.
+    Memcached has no way to make the pair atomic, so `increment` retries a
+    bounded number of times and then surfaces the loss instead of returning
+    `None` as if it were a count.
     """
     backend = stubbed_backend()
-    # INCR keeps missing: the key is gone again by the time ADD's retry runs.
+    # INCR keeps missing: the key is gone again by the time each retry runs.
     backend.client.incr.return_value = None
 
-    with pytest.raises(CacheXError, match="Counter vanished between ADD and INCR"):
-        await backend.increment("k")
+    with pytest.raises(
+        CacheXError,
+        match=f"Counter vanished between ADD and INCR on each of {_CAS_MAX_RETRIES} attempts",
+    ):
+        await backend.increment("k", ttl=1)
+
+    assert backend.client.add.call_count == _CAS_MAX_RETRIES
+    # The first INCR, then one after every ADD.
+    assert backend.client.incr.call_count == _CAS_MAX_RETRIES + 1
+
+
+async def test_increment_retries_when_the_new_counter_expires_before_incr() -> None:
+    """Issue #315: a ttl=1 counter can expire between its ADD and the INCR.
+
+    Memcached's clock has one-second resolution, so the tick can land between
+    the two. The counter expired inside its own window, so the retry starts
+    a new window at `delta`.
+    """
+    backend = stubbed_backend()
+    # Miss (no counter), miss (expired right after ADD), then the retry lands.
+    backend.client.incr.side_effect = [None, None, 3]
+
+    assert await backend.increment("k", 3, ttl=1) == 3
 
-    assert backend.client.add.call_count == 1
+    assert backend.client.add.call_count == 2
+    for call in backend.client.add.call_args_list:
+        assert call.args == ("fastapi_cachex:k", b"0", 1)
+    assert backend.client.incr.call_count == 3
 
 
 @requires_memcached