0.5.0 - EXPERIMENTAL#24
Merged
Merged
Conversation
The Redis fixtures flushed redis://localhost:6379/0 unconditionally, so running the suite on a machine whose default Redis belongs to another project would wipe that project's data. Tests now target database 15 by default and honor CASHET_TEST_REDIS_URL for full isolation.
put_blob wrote directly to the content-addressed path, so a crash mid-write left a truncated file that the exists() dedup check then treated as the real blob: every later put_blob of the same content skipped the write and every get_blob failed the integrity check, permanently poisoning that hash. Blobs are now written to a temp file and moved into place with os.replace, and get_blob deletes a file whose content does not match its content-derived path so the next put_blob can store it again.
A store error during a heartbeat renewal killed the heartbeat task, so the running claim stopped being renewed (letting another worker reclaim and double-run the task), and the finally block then re-raised the heartbeat's exception, destroying a result that had already been computed and leaving the commit stuck RUNNING. Renewals now retry on the next interval, and awaiting the heartbeat at shutdown logs instead of raising.
The per-fingerprint zset was scored by expires_at (infinity when no TTL was set), so lookups iterated by expiry with score ties broken by reverse-lexicographic hash. After a force re-run, later calls could serve the older result, and which commit won was effectively arbitrary, diverging from SQLite's created_at ordering. The zset is now scored by created_at; expiry filtering stays client-side as before. Legacy infinity-scored entries from pre-0.5.0 stores are detected on lookup and rescored in place so old indexes heal lazily. Parity tests pin the newest-wins and expired-newer cases on both stores.
evict() only considered last_accessed_at, so a commit whose TTL had expired was skipped on every lookup yet stayed on disk until it aged past the access cutoff (30 days by default), contradicting the documented behavior that expired entries are removed at garbage collection. SQLite eviction now also matches expires_at <= now; Redis maintains a cashet:index:expires zset and deletes everything scored at or below now. Redis commits written before 0.5.0 are not in the new index and still age out by access time only.
Set and frozenset arguments were ordered by raw repr, which embeds a memory address for objects without a custom repr. The same logical set could therefore serialize in a different order in another process and miss the cache even though every element hashes stably by state. Items are now serialized first and sorted by that stable form. Argument hashes change for sets containing custom objects or single-element tuples; those entries recompute on first access.
Every distinct fingerprint registered a FileLock in a process-global dict that was never evicted, and AsyncRedisStore cached one redis-py Lock object per fingerprint with the same unbounded growth. Long-lived workers leaked an entry per unique task submitted. SQLite fingerprint locks are now striped across 256 lock paths (a shared stripe only serializes rare claim sections), and Redis locks are constructed per acquisition, which also removes the token-state hazard of two coroutines sharing one Lock instance.
Every CLI command instantiated Client() eagerly, so running cashet log in any directory silently created an empty .cashet store there, and the only way to point the CLI at another store was the CASHET_DIR environment variable. Commands that inspect or mutate an existing store now exit non-zero with a clear message when the store directory does not exist; import and serve still create it. A group-level --store-dir option overrides the environment and the ./.cashet default.
A task raising an exception produced the same generic 500 Internal server error as a real server bug, so HTTP clients could not tell their own function's failure from a broken server, let alone debug it. Task failures now return 422 with the final traceback line (the exception type and message) while withholding the full traceback and its server file paths; unexpected server errors keep the generic 500.
Every cache hit rewrote the full commit row to flip its status to CACHED (information the executor already returns separately as was_cached), took the cross-process fingerprint lock just to read an immutable completed commit, and the claim path re-queried the fingerprint it had just missed to find a parent that therefore cannot exist. A hit is now a single fingerprint lookup: no lock, no commit rewrite, no redundant parent query. Stored commits keep their COMPLETED status permanently; the returned in-memory commit still reports CACHED. Forced and cache=False runs still query for parent lineage.
Every fingerprint lookup issued an UPDATE to refresh last_accessed_at, so each cache hit paid a write transaction and its WAL fsync (measured at 1.4ms) to feed an LRU whose eviction cutoffs are measured in days. The bump now fires only when the stored value is more than an hour stale, making steady-state hits pure reads. SQLite connections also use synchronous=NORMAL under WAL: a power loss can drop the newest commits, which a compute cache simply recomputes, and the database itself cannot corrupt.
build_task_def re-read the function's source (file IO through inspect), re-parsed it, and re-unparsed the AST on every submission, costing about 130us per call for a function whose identity cannot change while the object lives. Normalized source is now memoized per function object (weakly, so notebook redefinitions miss as new objects), AST canonicalization is memoized per source string, and stdlib/site path classification no longer rebuilds the site-packages list per lookup. Globals, defaults, and closures are still hashed live on every call, so mutated module state keeps invalidating as before. build_task_def drops to about 8us.
Median and minimum latency for hashing, sync and async cache hits, and the miss path, using only public API. Run with uv run python benchmarks/bench_hot_path.py to compare releases on the same machine.
Correctness and performance release. CHANGELOG.md carries the full list of fixes, the measured speedups, and the upgrade notes for shared stores.
store.py had grown to a thousand lines holding four concerns at once. Lock plumbing now lives in _locks.py, the table definition, migrations, and row mapping in _sqlite_schema.py, the storage engine in _sqlite_core.py (as SQLiteStoreCore), and store.py keeps only the public AsyncSQLiteStore and SQLiteStore wrappers, so the documented import path is unchanged. The hash-prefix validator moved to _ids.py so the Redis store can share it instead of keeping its own copy. Code moved verbatim; renames only drop leading underscores now that the symbols cross module boundaries.
hashing.py mixed function-identity hashing with four serializer classes that have nothing to do with hashing. They now live in cashet.serializers; cashet.hashing re-exports them so existing imports keep working, and the top-level cashet names are unchanged. Internal importers point at the new module.
redis_store.py interleaved the wire format (key naming scheme, commit JSON encoding, index pipeline commands, the delete Lua script) with the two store classes. The format helpers now live in _redis_codec.py and the module keeps only AsyncRedisStore and RedisStore, so the documented import path is unchanged. The store also uses the shared normalize_hash_prefix from _ids instead of carrying its own copy. Helpers renamed only to drop leading underscores now that they cross a module boundary.
Every endpoint existed twice: an async handler calling AsyncClient and a sync twin wrapping Client in a thread, duplicating request parsing, logging, and error mapping (~300 lines that had to be fixed in two places every time, as the 422 change just demonstrated). Handlers now run against a small ops adapter: _AsyncOps calls AsyncClient natively and _SyncOps wraps Client calls in asyncio.to_thread. create_app and create_async_app keep their signatures and share one app factory.
The claim path skipped the parent query with a condition that was only correct because find_existing_commit and find_parent_hash happened to run the same underlying query. One find_by_fingerprint inside the lock now serves both: it is the cached result when reuse is allowed and the parent for the new claim otherwise. find_parent_hash had no other callers and is gone.
Even a zero-row UPDATE acquires the writer lock, so a throttled hit racing a slow writer could stall up to busy_timeout before giving up on the access bump. The SELECT already returns last_accessed_at; only issue the UPDATE when the window has elapsed, keeping the SQL predicate as the guard against a concurrent bump. Eviction now collects hashes and blob refs in a single scan and deletes by hash: the expires_at OR defeats the last_accessed index, so the previous four statements ran four full table scans per gc. The unused _lock attribute and the unreachable None-connection branch in _delete_orphan_objects are removed.
Legacy fingerprint healing fetched every commit body one GET at a time, rewrote the zset, then re-read it and fetched the bodies again. It now uses one MGET and returns the rescored recency order directly. Expiry eviction batches stale index entries into a single ZREM, and commit_hash_from_key reuses decode_hash instead of reimplementing it.
_submit_options now emits the underscore-prefixed keywords that Client.submit and AsyncClient.submit accept, so both ops adapters splat them instead of unpacking six options key by key in two places. _build_app takes the client from the ops adapter rather than a second parameter, the gc handler converts days to timedelta once, and the handlers share one _ServerOps alias.
The setup deleted the blob ref counter through a hardcoded redis.Redis() (localhost database 0) while the store under test runs on the configurable test URL, so the missing-counter scenario was silently not simulated and the test passed vacuously. It now deletes through the store's own connection. The new fingerprint and TTL tests also share make_task_def/make_commit helpers instead of repeating the same scaffold.
expires_at is stamped when the claim is created, so a task that runs longer than its TTL is already expired while still executing. Expiry eviction deleted it regardless of status; a concurrent submission then found neither a result nor a running claim and executed the task a second time. The SQLite expiry predicate now excludes running and pending commits, and Redis only adds a commit to the expiry index once it reaches a terminal status. Access-age eviction stays status-blind so a crashed worker's abandoned claim still ages out.
Renewals fire every running_ttl / 2, so a renewal that fails halfway through the lease would next be attempted exactly at the staleness boundary; the retry also has to win the fingerprint lock first, so any contention pushed the claim past stale and another worker could reclaim and double-run a live task. A failed renewal now retries on a quarter of the interval, keeping the claim fresh unless failures persist.
A failed write left its temp file behind, and a crash between write and rename always did. Those files were counted by _blob_storage_totals as stored objects, inflating stats and giving size-based eviction a target it could never reach by deleting commits. Failed writes now remove their temp file, storage totals skip *.tmp, and eviction sweeps temp files older than an hour so a concurrent writer's in-flight file is never touched.
Redis hits update the shared access index with a ZADD on every lookup, so calling all hits read-only was wrong. The SQLite claim stands (steady-state hits perform no writes); the Redis behavior is stated as what it is, one cheap index update that feeds the shared LRU.
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.
What this is
Correctness and performance release, plus a module reorganization of the oversized files. Cache hits are now lock-free and 11-18x faster, six bug classes are fixed, and every fix ships with a regression test that was verified to fail before its fix. Marked experimental because it deliberately changes three behaviors and one storage setting, all listed below and in CHANGELOG.md.
Correctness fixes
put_blobwrote directly to the content-addressed path; the exists() dedup check then trusted the truncated file forever and every read failed the integrity check. Writes now go through a temp file plusos.replace,get_blobdeletes a corrupt file so the content can be stored again, failed writes remove their temp file, stats never count temp files, and gc sweeps hour-old crash leftovers.created_at, matching SQLite ordering; legacy entries are rescored lazily on first lookup. Parity tests pin both stores.expires_at <= now; Redis maintains acashet:index:expireszset. Running and pending commits are exempt: their TTL clock starts at claim time, so a task outliving its TTL must not be deleted mid-execution and re-run.Performance
Same committed script (
benchmarks/bench_hot_path.py), same machine, medians:build_task_def)How: cache hits stopped rewriting the commit row and taking the cross-process fingerprint lock; the SQLite access-time bump fires at most once per hour per commit instead of on every hit (Redis hits still pay one ZADD to the shared access index) (eviction cutoffs are measured in days, so LRU behavior is unchanged); SQLite runs
synchronous=NORMALunder WAL; function source resolution and AST canonicalization are memoized per function object while globals, defaults, and closures still hash live. Re-measured after the reorganization below: unchanged.Module reorganization (last five commits)
No public import path changes; code moved verbatim except where a symbol crossing a module boundary dropped its leading underscore.
store.py(1,045 lines, four concerns) becomes the public store classes only (179 lines); the engine is_sqlite_core.py, schema and migrations_sqlite_schema.py, lock plumbing_locks.py, and the hash-prefix validator_ids.py, shared with Redis instead of duplicated.server.py(741 lines) becomes 602, with the ~300 lines of duplicated sync/async handler pairs replaced by one handler set over an ops adapter (_AsyncOpsnative,_SyncOpsviaasyncio.to_thread). The 422 change earlier in this PR had to be written twice; this makes that class of drift impossible.hashing.pyintocashet.serializers;cashet.hashingre-exports them so existing imports keep working.redis_store.pybecomes store classes only; key scheme, commit codec, Lua script, and index pipeline commands live in_redis_codec.py.Deliberate behavior changes, please review
completed; thecachedflip is no longer persisted. The executor already reports hits through its return value, and no test or consumer relied on the stored flip./submitreturns 422 with the final exception line when a task raises, instead of a generic 500. Full tracebacks and server file paths are still withheld; 500 now always means the server itself failed. This re-draws the 0.4.5 hardening line, and the test pinning the old behavior was updated to pin the new split.importandservestill create). New group-level--store-diroption.synchronous=NORMAL: a power loss can drop the newest commits, which recompute; the database cannot corrupt.CASHET_TEST_REDIS_URL. The old fixtures flushed database 0 oflocalhost:6379unconditionally, which on a shared dev machine can belong to another project.Migration notes
Validation
ruff check,pyrightstrict, andpytestall clean after every commit: 402 passed, including the full Redis suite against a dedicated container.Known limitations left out on purpose
list_commitsfilters client-side, one round trip per commit when filtering large stores.ruff formatclean (18 files would reformat); left untouched to keep this diff reviewable. A format-only follow-up PR can fix that separately.