Problem
RedisCacheLayer.invalidate_by_tags() awaits two Redis commands per tag inside a
sequential for loop, so invalidating N tags costs 2N serial network round trips.
# src/youtube_extension/backend/services/intelligent_cache.py:509-519
for tag in tags:
keys = await conn.smembers(f"uvai:tag:{tag}") # round trip 1
if keys:
...
deleted = await conn.delete(*(cache_keys + stat_keys + [...])) # round trip 2
total_deleted += deleted
|
current |
after |
| Round trips for N tags |
2N, strictly serial |
2 waves, fan-out bounded by the existing limiter |
| 8 tags |
16 serial round trips |
2 waves |
| Event-loop starvation |
none |
unchanged |
The identical defect in the sibling method on the same class was fixed and merged
in #1152, "issue Redis tag-set writes concurrently on cache set". That PR added
_get_tag_write_semaphore() specifically to bound this kind of fan-out against the
shared connection pool. invalidate_by_tags() was left out of it and still pays the
serial cost — the limiter's own docstring names invalidate_by_tags() as one of the
paths it deliberately does not cover.
What is and is not claimed
This is a latency defect, not an event-loop starvation defect. await conn.smembers(...) yields control, so other coroutines keep running while a tag
invalidation is in flight. Nothing here freezes the loop, and no throughput
improvement for the running service is claimed.
The cost is borne by whoever awaits invalidate_by_tags(): its wall-clock latency
grows linearly with the tag count. A secondary consequence is that a longer
invalidation window is also a longer window during which entries that were meant to
be invalidated remain readable by get().
Reachability evidence
invalidate_by_tags() has no callers in src/ outside its own module:
$ grep -rnE "invalidate_by_tags|cache_invalidate_tags" src --include="*.py" \
| grep -v intelligent_cache.py
(no output)
It is reached only through the module's own public wrappers —
IntelligentCacheSystem.invalidate_by_tags() (L623) and the cache_invalidate_tags()
helper (L793) — neither of which is called from the request path either. An
import-closure walk from youtube_extension.main (60 modules) does not reach this
module at all.
This is therefore a library-surface fix. It is worth making because the method is
public API that callers are expected to use, and because leaving one half of a class
serial while its sibling is concurrent is an inconsistency that invites the same bug
to be reintroduced. It is not worth claiming as a production latency win.
Acceptance criteria
Proposed fix
Apply the merged #1152 pattern to invalidate_by_tags(): move the per-tag body into a
local coroutine that acquires self._get_tag_write_semaphore(), gather the tasks with
return_exceptions=True, re-raise the first exception so the existing handler still
returns 0, and sum the rest.
Hold the permit across both commands for a tag, not one each. The delete depends
on the keys returned by smembers, so they are causally ordered; one permit per tag
keeps the number of concurrently held pool connections equal to the permit count.
Known behaviour change to disclose
On the failure path the serial loop stops at the first failing tag, leaving later tags
untouched. The concurrent version has already issued them, so more tags may be
invalidated before the error surfaces. The reported return value is identical (0 in
both cases). For an invalidation operation this is the safe direction to err in —
over-invalidating costs a cache miss, whereas under-invalidating leaves stale entries
readable.
Problem
RedisCacheLayer.invalidate_by_tags()awaits two Redis commands per tag inside asequential
forloop, so invalidating N tags costs 2N serial network round trips.2N, strictly serial2waves, fan-out bounded by the existing limiterThe identical defect in the sibling method on the same class was fixed and merged
in #1152, "issue Redis tag-set writes concurrently on cache set". That PR added
_get_tag_write_semaphore()specifically to bound this kind of fan-out against theshared connection pool.
invalidate_by_tags()was left out of it and still pays theserial cost — the limiter's own docstring names
invalidate_by_tags()as one of thepaths it deliberately does not cover.
What is and is not claimed
This is a latency defect, not an event-loop starvation defect.
await conn.smembers(...)yields control, so other coroutines keep running while a taginvalidation is in flight. Nothing here freezes the loop, and no throughput
improvement for the running service is claimed.
The cost is borne by whoever awaits
invalidate_by_tags(): its wall-clock latencygrows linearly with the tag count. A secondary consequence is that a longer
invalidation window is also a longer window during which entries that were meant to
be invalidated remain readable by
get().Reachability evidence
invalidate_by_tags()has no callers insrc/outside its own module:It is reached only through the module's own public wrappers —
IntelligentCacheSystem.invalidate_by_tags()(L623) and thecache_invalidate_tags()helper (L793) — neither of which is called from the request path either. An
import-closure walk from
youtube_extension.main(60 modules) does not reach thismodule at all.
This is therefore a library-surface fix. It is worth making because the method is
public API that callers are expected to use, and because leaving one half of a class
serial while its sibling is concurrent is an inconsistency that invites the same bug
to be reintroduced. It is not worth claiming as a production latency win.
Acceptance criteria
invalidate_by_tags()is issued concurrently rather than one tag per serial await.self.redis_pool.deleteresults on success,0on failure,0when disconnected.invalidate_*unit tests pass unmodified.invalidate_by_tags()is not covered by it.Proposed fix
Apply the merged #1152 pattern to
invalidate_by_tags(): move the per-tag body into alocal coroutine that acquires
self._get_tag_write_semaphore(), gather the tasks withreturn_exceptions=True, re-raise the first exception so the existing handler stillreturns
0, and sum the rest.Hold the permit across both commands for a tag, not one each. The
deletedependson the keys returned by
smembers, so they are causally ordered; one permit per tagkeeps the number of concurrently held pool connections equal to the permit count.
Known behaviour change to disclose
On the failure path the serial loop stops at the first failing tag, leaving later tags
untouched. The concurrent version has already issued them, so more tags may be
invalidated before the error surfaces. The reported return value is identical (
0inboth cases). For an invalidation operation this is the safe direction to err in —
over-invalidating costs a cache miss, whereas under-invalidating leaves stale entries
readable.