fix(appkit): honor TTL in CacheManager.getOrExecute#326
Open
Tim-Hoare wants to merge 1 commit intodatabricks:mainfrom
Open
fix(appkit): honor TTL in CacheManager.getOrExecute#326Tim-Hoare wants to merge 1 commit intodatabricks:mainfrom
Tim-Hoare wants to merge 1 commit intodatabricks:mainfrom
Conversation
CacheManager.getOrExecute calls storage.get() and returns the cached
value without checking expiry. InMemoryStorage.get() returns entries
unconditionally by design ("expiry check is done by CacheManager"),
but the check was missing on the getOrExecute path — only the
standalone CacheManager.get() honored TTL.
With the in-memory backend (the default when Lakebase is not
configured), this caused cached entries to live until LRU eviction at
maxSize or process restart, regardless of the configured ttl. Apps
using the analytics plugin's default cache (enabled, ttl: 3600) would
serve stale query results indefinitely.
Add an explicit expiry check in getOrExecute: if storage returns an
expired entry, delete it and fall through to a normal cache miss so
fn() re-executes.
Adds a regression test in the getOrExecute describe block. The
existing TTL test only covered the set/get pair, which is why this
was not caught.
Fixes databricks#325
Signed-off-by: Tim Hoare <t.hoare@codat.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #325.
Problem
CacheManager.getOrExecutereturns cached entries without checking expiry, so whenInMemoryStorageis the backend (the default when Lakebase isn't configured) cached values are served indefinitely — until LRU eviction atmaxSizeor process restart. The configuredttlis silently ignored on this code path.This affects every AppKit app using the analytics plugin's default cache (
enabled: true, ttl: 3600) without Lakebase. In a deployed Databricks App we observed query results staying cached for >19 hours despite a 1-hour TTL.Why it slipped through
InMemoryStorage.get()is intentionally a raw lookup — the existing testshould return expired entry on get (expiry check is done by CacheManager)makes that contract explicit. The standaloneCacheManager.get()honors it correctly. ButCacheManager.getOrExecutedidn't, and the existing TTL test ("should respect TTL expiry") only covered theset/getpair, notgetOrExecute.Fix
In
getOrExecute, afterstorage.get(cacheKey)returns an entry, checkDate.now() > cached.expiry. If expired: delete it and fall through to the normal cache-miss path sofn()re-executes. Otherwise return the cached value as before.Net change is small and stays within the existing "expiry handling lives at the CacheManager layer" contract — no signature changes, no storage changes.
Test
Added a regression test in the
describe("getOrExecute", …)block: caches a value with a 1ms TTL, waits, callsgetOrExecuteagain, and asserts thatfnwas invoked twice and returns the new value.Confirmed:
expected 'result-1' to be 'result-2').appkitpackage tests continue to pass.Lint (
biome check) and typecheck (tsc --noEmit) clean.Notes
getValid()helper used by bothgetandgetOrExecute) if a different shape is preferred.