Skip to content

Commit 74937d5

Browse files
authored
fix(decorators): key and name the registry set by a str namespace's exact value (LAB-6197) (#388)
1 parent 0722b08 commit 74937d5

3 files changed

Lines changed: 208 additions & 6 deletions

File tree

‎docs/features/l1-invalidation.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,13 @@ Things to know:
210210

211211
**Backend failures.** Invalidation never raises on a backend failure: `invalidate_cache()` and `ainvalidate_cache()` return `None` whether or not the L2 deletes succeed. (The interop prefix guard is not a backend failure and still raises its `ConfigurationError`; see [Interop Mode](interop-mode.md).) When a no-args invalidation deletes this process's keys one by one — on a backend without a key registry, or after a failed drain — and some of those deletes fail, cachekit logs one ERROR `Failed to delete N L2 key(s)` per call, with the count, rather than one line per key. Each of those keys stays tracked in this process, so this process's next no-args `invalidate_cache()` retries it. Until then, the entries are still served from L2.
212212

213+
**`str` subclass namespaces.** A `str` subclass namespace is used as its plain `str` value everywhere, so a `StrEnum` or `(str, Enum)` member `USERS = "users"` is the namespace `users`. Earlier releases rendered a `(str, Enum)` member as `NS.USERS` in some places: in metrics labels on every Python version, and in custom `key=` keys, key registry set names and log-message prefixes on Python 3.11 and later. Plain `str` and `StrEnum` namespaces are unaffected. For a `(str, Enum)` namespace, upgrading changes the following.
214+
215+
- **Custom `key=` entries move (Python 3.11+).** They move from `NS.USERS:k` to `users:k`, and the old entries are no longer served, so each distinct key is recomputed once. On a function that takes parameters, one no-args `invalidate_cache()` from an upgraded process deletes every old entry still tracked in the old registry set; run it once per tenant, since it reaches only the tenant set in `tenant_context`. On a zero-parameter function, a no-args `invalidate_cache()` deletes only the new `users:k` entry and never drains a registry set, so it does not erase the old one. Old entries that are not tracked, because their set expired seven days after its last write or because the backend does not track keys, and every zero-parameter function's old entry, retire only by TTL, and never if none was set. To erase them, run the `scan_iter` + `unlink` script from [Upgrading to 0.20.0](../backends/README.md#upgrading-to-0200) with `pattern = "t:*:NS.USERS:*"` on the tenant-scoped Redis backend (env auto-detection or `RedisBackendProvider`), or `pattern = "NS.USERS:*"` on a plain `RedisBackend`. A `redis-cli SCAN NS.USERS:*` against the tenant-scoped backend matches nothing, because its keys carry the `t:{tenant}:` prefix.
216+
- **The key registry set is renamed (Python 3.11+).** It moves from `ck:reg:NS.USERS:…` to `ck:reg:users:…`. Auto-mode keys do not move. On a function that takes parameters, a no-args `invalidate_cache()` drains both sets, so entries tracked before the upgrade are still invalidated. If the old set's drain fails, cachekit logs a WARNING `Legacy key registry drain failed` and still applies the new set's drain; the old set's entries that the failed drain had not yet deleted stay in it for the next no-args `invalidate_cache()`. A drain that fails part-way through a set larger than 10 000 keys has already deleted some of them from L2, and other wrappers of the same function in this process can keep serving their L1 copies of those keys until the L1 TTL, as when the new set's drain fails. A zero-parameter function has a single auto-mode key, which its no-args `invalidate_cache()` deletes directly.
217+
- **Rolling deploys and rollbacks leave stale entries (Python 3.11+).** During a rollout, a no-args `invalidate_cache()` from a process on the earlier release misses entries the upgraded processes serve. On a function that takes parameters, it drains only the old set, so it misses every entry tracked in the new one. On a zero-parameter function, it deletes only the key the earlier release derives: in auto mode that is the shared key, so nothing is missed, but with `key=` it deletes `NS.USERS:k` and leaves `users:k`. Missed entries stay stale after the rollout completes, until their TTL (never, if none was set) or until an upgraded process invalidates them. Once the rollout completes, run one no-args `invalidate_cache()` per tenant from an upgraded process for each affected function. A rollback leaves the same gap in the other direction for entries tracked in the new set, which lives seven days after its last write. For `key=` functions, the earlier release also serves `NS.USERS:k` entries again, which this release's exact-args and zero-parameter invalidations never deleted. After a rollback, run one no-args `invalidate_cache()` per tenant from a one-off script on this release, then erase the old `key=` entries with the `scan_iter` + `unlink` script above.
218+
- **Observability names change.** The `namespace` metrics label reads `users` instead of `NS.USERS` on every Python version, 3.10 included, so update dashboards and alerts that match on it. On Python 3.11 and later, the log-message prefix `[NS.USERS]` becomes `[users]`.
219+
213220
**Tenant scope:** with the tenant-scoped Redis backend (env auto-detection, or `RedisBackendProvider(...).get_shared_backend()`), each tenant's entries live under its own `t:{tenant}:` prefix. `invalidate_cache()` — with or without arguments — deletes only the L2 entries of the tenant set in `tenant_context` for the calling context (`default` when none is set); other tenants' entries stay cached and tracked. L1 is not tenant-scoped: within a process, all tenants share one L1 entry per cache key, so a tenant can be served the value another tenant cached, and `invalidate_cache()` evicts that entry for every tenant. Disable L1 on functions whose results differ by tenant: `@cache(..., l1_enabled=False)`, or with a preset `@cache.production(..., l1_enabled=False)`, which keeps the preset's other L1 settings.
214221

215222
---

‎src/cachekit/decorators/wrapper.py‎

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -583,11 +583,20 @@ def create_cache_wrapper(
583583

584584
func_hash = function_hash(f"{func.__module__}.{func.__qualname__}")
585585

586+
# Rebind a str namespace to its exact str value before any use (LAB-6197). A str
587+
# subclass renders through its own __format__/__eq__/startswith: a (str, Enum) member
588+
# formats as "NS.USERS" on Python 3.11+ but "users" on 3.10, which split registry ids and
589+
# key= keys across versions and could slip a crafted value past the "ck" check below.
590+
# _legacy_namespace keeps the pre-fix f-string rendering for the registry drain below.
591+
_legacy_namespace: str | None = None
592+
if isinstance(namespace, str):
593+
_legacy_namespace = f"{namespace}"
594+
namespace = str.__str__(namespace)
595+
586596
# INTEROP MODE (interop/v1, protocol spec/interop-mode.md): validate loudly at
587597
# decoration time. These checks also cover direct create_cache_wrapper callers
588-
# that bypass DecoratorConfig validation. Runs before any other use of namespace
589-
# and rebinds both segments to the exact str values it checked, so a str subclass
590-
# (e.g. a (str, Enum) member) cannot render differently in a key.
598+
# that bypass DecoratorConfig validation. Rebinds interop to the exact str value it
599+
# checked; namespace is already exact from the block above.
591600
_interop_sig: inspect.Signature | None = None
592601
if interop is not None:
593602
try:
@@ -616,9 +625,19 @@ def create_cache_wrapper(
616625
# a key written under it could take the ck:reg: shape and overwrite a tracking set.
617626
if namespace == "ck" or (namespace or "").startswith("ck:"):
618627
raise ConfigurationError("namespace 'ck' (and 'ck:*') is reserved for cachekit's key registry")
619-
_registry_id = (
620-
f"ck:reg:{namespace if namespace is not None else ''}:"
621-
f"{blake3_hash(f'{func.__module__}.{func.__qualname__}', digest_size=8)}"
628+
_registry_hash = blake3_hash(f"{func.__module__}.{func.__qualname__}", digest_size=8)
629+
_registry_id = f"ck:reg:{namespace if namespace is not None else ''}:{_registry_hash}"
630+
# Pre-fix releases named the set with the namespace's f-string rendering. Auto-mode keys
631+
# did not move, so entries tracked under the old name are still served; the no-args drain
632+
# empties that set too, or they would outlive invalidate_cache() (LAB-5288 precedent).
633+
# Interop is skipped: 0.20.0 shipped the registry with interop's exact-str rebind, so no
634+
# release wrote a non-exact interop set. This drains the set name 0.20.x wrote: remove
635+
# it only in a major release whose notes declare upgrades from 0.20.x unsupported (the
636+
# rule get_legacy_cache_key follows).
637+
_legacy_registry_id = (
638+
f"ck:reg:{_legacy_namespace}:{_registry_hash}"
639+
if interop is None and _legacy_namespace is not None and _legacy_namespace != namespace
640+
else None
622641
)
623642

624643
# ENCRYPTION + L1-ONLY (LAB-4665, protocol spec/intent-presets.md § L1 Posture rule 3:
@@ -2336,6 +2355,14 @@ def _drain_all() -> None:
23362355
scope = _l2_scope()
23372356
mine = {entry for entry in snap if entry[0] == scope}
23382357
deleted = _backend.drain_tracked(_registry_id, {key for _, key in mine}) # type: ignore[union-attr]
2358+
if _legacy_registry_id is not None:
2359+
# Its own try: the primary drain already deleted keys that other wrappers
2360+
# may hold in the shared L1, so its result must still be applied below.
2361+
# A failed legacy drain leaves its members in the old set for the next one.
2362+
try:
2363+
deleted |= _backend.drain_tracked(_legacy_registry_id, ()) # type: ignore[union-attr]
2364+
except Exception as e:
2365+
_logger.warning("Legacy key registry drain failed: %s", redact_error_for_log(e))
23392366
# Trim BEFORE evicting: _put_l1 puts then records, so a concurrent write can
23402367
# never leave an L1 entry whose key is no longer in _cached_keys.
23412368
trim = mine - watch

‎tests/unit/test_key_registry.py‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@
1313
import contextvars
1414
import logging
1515
import os
16+
import sys
1617
import threading
1718
import time
19+
from enum import Enum
1820
from pathlib import Path
1921
from typing import Any, Optional
2022

@@ -797,3 +799,169 @@ def __exit__(self, *exc: object) -> None:
797799
l1._lock = real_lock
798800
assert acquisitions == 3 # 1 000-key batches: a large drain never holds every get/put off at once
799801
assert l1.get("k2499") == (False, None)
802+
803+
804+
class _NS(str, Enum):
805+
USERS = "users"
806+
807+
808+
class _StrEnumNS(str, Enum):
809+
"""enum.StrEnum's rendering (3.11+), spelled out so the test also runs on 3.10."""
810+
811+
USERS = "users"
812+
__str__ = str.__str__
813+
__format__ = str.__format__
814+
815+
816+
class _FormatsAs(str):
817+
"""A str whose __format__ lies: f-strings render ``rendered``, "".join the real value."""
818+
819+
rendered = "ck:reg:users"
820+
821+
def __format__(self, spec: str) -> str:
822+
return self.rendered
823+
824+
825+
class _LegacyFormat(_FormatsAs):
826+
rendered = "legacy"
827+
828+
829+
class _HidesCk(str):
830+
"""A str that claims not to be "ck" and not to start with it."""
831+
832+
def __eq__(self, other: object) -> bool:
833+
return False
834+
835+
__hash__ = str.__hash__
836+
837+
def startswith(self, *args: Any, **kwargs: Any) -> bool: # type: ignore[override]
838+
return False
839+
840+
841+
@pytest.mark.unit
842+
class TestNamespaceExactStr:
843+
"""A str-subclass namespace keys and names its registry set by its underlying str (LAB-6197)."""
844+
845+
def test_str_enum_registry_id_uses_value(self) -> None:
846+
backend = TrackingBackend()
847+
848+
@cache(backend=backend, ttl=60, namespace=_NS.USERS, l1_enabled=False)
849+
def f(x: int) -> int:
850+
return x
851+
852+
f(1)
853+
(rid,) = _registry_ids(backend)
854+
assert rid.startswith("ck:reg:users:")
855+
856+
def test_str_enum_custom_key_uses_value(self) -> None:
857+
backend = TrackingBackend()
858+
859+
@cache(backend=backend, ttl=60, namespace=_NS.USERS, key=lambda *a, **kw: "k", l1_enabled=False)
860+
def f(x: int) -> int:
861+
return x
862+
863+
f(1)
864+
assert set(backend.store) == {"users:k"}
865+
(rid,) = _registry_ids(backend)
866+
assert rid.startswith("ck:reg:users:")
867+
868+
def test_format_override_cannot_forge_registry_shape(self) -> None:
869+
backend = TrackingBackend()
870+
871+
@cache(backend=backend, ttl=60, namespace=_FormatsAs("x"), key=lambda *a, **kw: "k", l1_enabled=False)
872+
def f(x: int) -> int:
873+
return x
874+
875+
f(1)
876+
assert set(backend.store) == {"x:k"}
877+
(rid,) = _registry_ids(backend)
878+
assert rid.startswith("ck:reg:x:")
879+
880+
@pytest.mark.parametrize("value", ["ck", "ck:reg"])
881+
def test_eq_and_startswith_override_cannot_bypass_ck_reservation(self, value: str) -> None:
882+
ns = _HidesCk(value)
883+
assert not ns == "ck" and not ns.startswith("ck:") # the overrides the old check trusted
884+
with pytest.raises(ConfigurationError, match="reserved"):
885+
886+
@cache(backend=TrackingBackend(), ttl=60, namespace=ns)
887+
def f(x: int) -> int:
888+
return x
889+
890+
def test_drain_also_empties_pre_fix_registry_set(self) -> None:
891+
"""Entries tracked under the pre-fix f-string registry id go on a no-args drain."""
892+
backend = TrackingBackend()
893+
894+
@cache(backend=backend, ttl=60, namespace=_LegacyFormat("x"), l1_enabled=False)
895+
def f(x: int) -> int:
896+
return x
897+
898+
f(1)
899+
(rid,) = _registry_ids(backend)
900+
legacy_rid = "ck:reg:legacy:" + rid.rsplit(":", 1)[1]
901+
backend.store["pre-upgrade-key"] = b"x" # written and tracked by pre-fix code
902+
backend.sets[legacy_rid] = {"pre-upgrade-key"}
903+
904+
f.invalidate_cache()
905+
assert backend.store == {}
906+
assert [r for r, _ in backend.drain_calls] == [rid, legacy_rid]
907+
908+
def test_legacy_drain_failure_still_applies_primary_drain(self, caplog: pytest.LogCaptureFixture) -> None:
909+
"""A failed legacy drain must not discard what the primary drain deleted: another
910+
wrapper's shared-L1 copy of a drained key is still evicted, and the old set is kept."""
911+
912+
class LegacyFails(TrackingBackend):
913+
def drain_tracked(self, registry_id: str, local_keys: Any) -> set[str]:
914+
if registry_id.startswith("ck:reg:legacy:"):
915+
raise BackendError("legacy drain failed")
916+
return super().drain_tracked(registry_id, local_keys)
917+
918+
backend = LegacyFails()
919+
calls: list[int] = []
920+
921+
def f(x: int) -> int:
922+
calls.append(x)
923+
return x
924+
925+
ns = _LegacyFormat("legacy_fail")
926+
writer = cache(backend=backend, ttl=60, namespace=ns)(f)
927+
writer(1) # this wrapper's L1 now holds the entry
928+
(rid,) = _registry_ids(backend)
929+
legacy_rid = "ck:reg:legacy:" + rid.rsplit(":", 1)[1]
930+
backend.sets[legacy_rid] = {"pre-upgrade-key"}
931+
932+
fresh = cache(backend=backend, ttl=60, namespace=ns)(f) # knows no keys itself
933+
with caplog.at_level(logging.WARNING):
934+
fresh.invalidate_cache()
935+
936+
assert "Legacy key registry drain failed" in caplog.text
937+
assert "invalidating local keys only" not in caplog.text
938+
assert legacy_rid in backend.sets # retried by the next drain
939+
writer(1)
940+
assert calls == [1, 1] # the shared L1 copy was evicted, so it recomputed
941+
942+
@pytest.mark.parametrize(
943+
("namespace", "interop", "legacy"),
944+
[
945+
("users", None, None),
946+
(_StrEnumNS.USERS, None, None),
947+
(None, None, None),
948+
# f"{member}" is "_NS.USERS" only from 3.11; on 3.10 the set name never moved.
949+
(_NS.USERS, None, "_NS.USERS" if sys.version_info >= (3, 11) else None),
950+
# 0.20.0 shipped the registry with interop's exact-str rebind: no pre-fix set exists.
951+
(_NS.USERS, "get_user", None),
952+
],
953+
)
954+
def test_no_args_drain_ids(self, namespace: Optional[str], interop: Optional[str], legacy: Optional[str]) -> None:
955+
"""Only a namespace whose set name actually moved gets a second drain."""
956+
backend = TrackingBackend()
957+
958+
@cache(backend=backend, ttl=60, namespace=namespace, interop=interop, l1_enabled=False)
959+
def f(x: int) -> int:
960+
return x
961+
962+
f(1)
963+
(rid,) = _registry_ids(backend)
964+
assert rid.startswith(f"ck:reg:{'users' if namespace is not None else ''}:")
965+
f.invalidate_cache()
966+
expected = [rid] if legacy is None else [rid, f"ck:reg:{legacy}:{rid.rsplit(':', 1)[1]}"]
967+
assert [r for r, _ in backend.drain_calls] == expected

0 commit comments

Comments
 (0)