From 131a382d597e285f3ca1e2a080e5aa931e3788b9 Mon Sep 17 00:00:00 2001 From: allen0099 Date: Sat, 26 Sep 2026 16:07:46 +0000 Subject: [PATCH] fix(memcached): raise while a server is unreachable instead of returning defaults With pymemcache's default retries, HashClient answered every call to a server that had just failed with the command's default until the retry was due: get() looked like a miss, set() dropped the write, increment() returned 0 and delete_if_equals() raised TypeError on the missing CAS pair. The client now uses retry_attempts=0, so a failed server leaves rotation at once and calls raise, and dead_timeout=1, so it is tried again after one second rather than 60. Closes #197 --- CHANGELOG.md | 7 +++ docs/BACKENDS.md | 7 +++ fastapi_cachex/backends/memcached.py | 13 ++++++ i18n/zh-TW/docs/BACKENDS.md | 2 + tests/backends/test_memcached.py | 67 ++++++++++++++++++++++++++++ 5 files changed, 96 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f667a32..6314a8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -147,6 +147,13 @@ Note that 0.3.3 was never released; 0.3.4 follows 0.3.2. again. The task is now started again on the current loop, and a task left on a loop that is still open is cancelled there. ([#181](https://github.com/allen0099/FastAPI-CacheX/issues/181)) +- **`MemcachedBackend` raises while a server is unreachable instead of + returning made-up results.** pymemcache's default retries answered calls in + the second after a failure with each command's default, so `get()` looked + like a miss, `set()` dropped the write, `increment()` returned 0 and + `delete_if_equals()` raised `TypeError`. A failed server is now taken out of + rotation at once and tried again after one second, instead of after 60. + ([#197](https://github.com/allen0099/FastAPI-CacheX/issues/197)) ## [0.3.7] - 2026-09-25 diff --git a/docs/BACKENDS.md b/docs/BACKENDS.md index d2120ea..a88086c 100644 --- a/docs/BACKENDS.md +++ b/docs/BACKENDS.md @@ -149,6 +149,13 @@ so concurrent requests never share a socket. Writes wait for the server's acknowledgement (`default_noreply=False`), which keeps a value readable from any pooled connection as soon as `set()` returns. +When a server cannot be reached, every call that would go to it raises, and the +server is tried again after one second. Before 0.3.8, calls in the second after a +failure returned made-up results instead: `get()` a miss, `set()` nothing (the write +was lost), `increment()` a fresh counter of 0. With several servers, a failed one is +taken out of rotation at once and its keys go to the remaining servers until it +answers again. + ## Atomic backend primitives Every backend exposes atomic operations on top of `get`/`set`/`delete`, for diff --git a/fastapi_cachex/backends/memcached.py b/fastapi_cachex/backends/memcached.py index 2dde101..8db9397 100644 --- a/fastapi_cachex/backends/memcached.py +++ b/fastapi_cachex/backends/memcached.py @@ -29,6 +29,10 @@ # not as a duration, so a longer TTL has to be converted before it is sent. _MAX_RELATIVE_TTL = 30 * 24 * 60 * 60 +# Seconds a failed server stays out of rotation before HashClient tries it +# again. Until then every call to it raises. +_DEAD_TIMEOUT = 1 + def _expiry(ttl: int | None) -> int: """Convert a TTL in seconds to the exptime Memcached expects. @@ -84,12 +88,21 @@ def __init__( # every write waits for the server's acknowledgement; otherwise a # ``set`` on one socket may still be in flight when a ``get`` on another # socket is served, and the caller would miss its own write. + # + # With retries enabled, HashClient answers calls to a server that just + # failed with each command's default (``None``, ``False``, ``0``) until + # the retry is due, which reads as a miss, a lost race or a fresh + # counter. ``retry_attempts=0`` takes a failed server out of rotation + # at once instead, so calls raise until it is tried again after + # ``_DEAD_TIMEOUT`` seconds. self.client = HashClient( servers, connect_timeout=5, timeout=5, use_pooling=True, default_noreply=False, + retry_attempts=0, + dead_timeout=_DEAD_TIMEOUT, ) self.key_prefix = key_prefix diff --git a/i18n/zh-TW/docs/BACKENDS.md b/i18n/zh-TW/docs/BACKENDS.md index cf086ac..99bba7a 100644 --- a/i18n/zh-TW/docs/BACKENDS.md +++ b/i18n/zh-TW/docs/BACKENDS.md @@ -111,6 +111,8 @@ BackendProxy.set(backend) 同步的 pymemcache 用戶端在工作執行緒中執行，並使用連線池，因此並行的請求絕不會共用同一個 socket。寫入會等待伺服器確認（`default_noreply=False`），因此只要 `set()` 返回，就能從連線池中的任何連線讀到該值。 +伺服器無法連線時，所有要送往它的呼叫都會拋出錯誤，一秒後會再嘗試連線。0.3.8 以前，失敗後一秒內的呼叫會回傳虛構的結果：`get()` 當成未命中、`set()` 沒有任何反應（寫入遺失）、`increment()` 當成新的計數器並回傳 0。設定多台伺服器時，失敗的那台會立即移出輪替，它的鍵會改由其餘伺服器處理，直到它恢復回應。 + ## 後端的原子操作 {#atomic-backend-primitives} 每個後端都在 `get`／`set`／`delete` 之上提供原子操作，供會被許多並行請求讀寫的值使用： diff --git a/tests/backends/test_memcached.py b/tests/backends/test_memcached.py index 2bcbbfc..8956044 100644 --- a/tests/backends/test_memcached.py +++ b/tests/backends/test_memcached.py @@ -7,6 +7,7 @@ import pytest_asyncio from fastapi_cachex.backends import MemcachedBackend +from fastapi_cachex.backends.memcached import _DEAD_TIMEOUT from fastapi_cachex.exceptions import CacheXError from fastapi_cachex.lock import CacheLock from fastapi_cachex.types import CacheEntry @@ -264,6 +265,72 @@ async def test_memcached_clear_path_raises_when_the_server_is_unreachable() -> N await backend.clear_path("/nope") +def _unreachable_backend() -> MemcachedBackend: + with socket.socket() as probe: + probe.bind(("127.0.0.1", 0)) + closed_port = probe.getsockname()[1] + return MemcachedBackend(servers=[f"127.0.0.1:{closed_port}"]) + + +_ENTRY = CacheEntry(fingerprint="f", content=b"x") + + +@pytest.mark.parametrize( + "call", + [ + lambda b: b.get("k"), + lambda b: b.set("k", _ENTRY), + lambda b: b.set_if_absent("k", _ENTRY), + lambda b: b.increment("k"), + lambda b: b.get_and_delete("k"), + lambda b: b.delete_if_equals("k", _ENTRY), + lambda b: b.expire_if_equals("k", _ENTRY, 5), + lambda b: b.delete_many(["a", "b"]), + lambda b: b.delete("k"), + lambda b: b.clear_path("/k"), + ], + ids=[ + "get", + "set", + "set_if_absent", + "increment", + "get_and_delete", + "delete_if_equals", + "expire_if_equals", + "delete_many", + "delete", + "clear_path", + ], +) +@pytest.mark.asyncio +async def test_memcached_keeps_raising_after_a_connection_failure(call) -> None: + """A call right after a failed one raises instead of returning a default (#197). + + With pymemcache's default retries, `HashClient` answered every call to a + server that had just failed with `None`/`False`/`0` for about a second, so + `increment` returned 0 and `set` silently dropped the write. + """ + backend = _unreachable_backend() + with pytest.raises(ConnectionRefusedError): + await backend.get("trip") + + with pytest.raises(Exception, match="All servers seem to be down"): + await call(backend) + + +@pytest.mark.asyncio +async def test_memcached_tries_a_failed_server_again_after_the_dead_timeout() -> None: + """A failed server is not skipped for pymemcache's default 60 seconds (#197).""" + backend = _unreachable_backend() + with pytest.raises(ConnectionRefusedError): + await backend.get("trip") + + await asyncio.sleep(_DEAD_TIMEOUT + 0.1) + + with pytest.raises(ConnectionRefusedError): + await backend.get("k") + + @requires_memcached @pytest.mark.asyncio async def test_memcached_clear_path_propagates_client_errors(