fix(caching): apply each pipeline operation's own ttl in InMemoryCache

async_increment_pipeline dropped each RedisPipelineIncrementOperation's own
ttl field, so a counter created through it (Router's TPM/RPM tracking,
parallel_request_limiter_v3's token/dollar accounting when Redis is absent,
and this PR's own tag-based token/dollar limits) always fell back to the
cache's 600-second default_ttl regardless of a real, often much longer,
configured window. An hourly or daily limit's counter would silently expire
and reset mid-window. allow_ttl_override already leaves a still-live ttl
untouched on a later call, so threading the operation's ttl through on every
increment only ever takes effect the first time. bugbot caught this on review.
This commit is contained in:
Deepanshu 2026-08-26 16:26:11 -04:00
parent 513c273750
commit 2ae037856a
2 changed files with 42 additions and 1 deletions

View file

@ -248,7 +248,15 @@ class InMemoryCache(BaseCache):
) -> list[float] | None:
results: Final = []
for increment in increment_list:
result = await self.async_increment(increment["key"], increment["increment_value"], **kwargs)
# Each operation's own ttl must reach set_cache, or a key with no
# live ttl yet falls through to the cache's short default_ttl
# instead of the caller's real (often much longer) window --
# allow_ttl_override already leaves an existing, still-live ttl
# untouched on a later increment, so passing this through on
# every call is safe: it only ever takes effect the first time.
result = await self.async_increment(
increment["key"], increment["increment_value"], ttl=increment.get("ttl"), **kwargs
)
results.append(result)
return results

View file

@ -45,6 +45,39 @@ async def test_async_increment_delegates_to_locked_sync_path():
assert cache.get_cache("counter") == 5
async def test_async_increment_pipeline_applies_each_operations_own_ttl():
"""
Bugbot finding: async_increment_pipeline dropped each operation's own
"ttl" field, so a counter created through it always got the cache's
600-second default_ttl regardless of what the caller actually configured
(e.g. an hourly or daily rate-limit window) -- the counter (and whatever
it was tracking against a limit) silently reset mid-window.
"""
cache = InMemoryCache(default_ttl=600)
await cache.async_increment_pipeline([{"key": "long-window-counter", "increment_value": 1, "ttl": 7200}])
ttl_remaining = await cache.async_get_ttl("long-window-counter")
assert ttl_remaining is not None
assert ttl_remaining > time.time() + 600
async def test_async_increment_pipeline_preserves_an_existing_live_ttl_on_later_increments():
"""
A second increment on the same still-live counter must not reset its
remaining ttl back up to the full window -- only the first increment
(the one that actually creates the counter) should set it.
"""
cache = InMemoryCache(default_ttl=600)
await cache.async_increment_pipeline([{"key": "counter", "increment_value": 1, "ttl": 7200}])
ttl_after_first = cache.ttl_dict["counter"]
await cache.async_increment_pipeline([{"key": "counter", "increment_value": 1, "ttl": 7200}])
ttl_after_second = cache.ttl_dict["counter"]
assert ttl_after_second == ttl_after_first
assert cache.get_cache("counter") == 2
def test_in_memory_openai_obj_cache():
from openai import OpenAI