Persistent, content-addressed context store + idle-time prewarming - #231
Persistent, content-addressed context store + idle-time prewarming#231cpsievert wants to merge 10 commits into
Conversation
commons_server() used to kick off pre-warming itself; with it gone, export a helper so custom apps get the same behavior with one call. commons_prewarm(agent) validates the agent and defers prewarm() to post-startup idle time, and is used by commons_app() and throughout the examples, vignette, and onboarding skill. The error contract is split by call site. A direct agent$prewarm() is typically warming caches ahead of deployment, so failures propagate: a cold cache should fail the deploy, and a warning would sail through a deploy script. commons_prewarm() downgrades failures to warnings, since pre-warming is a pure optimization (everything it builds is rebuilt lazily at first use) and an error escaping a later::later() callback would stop the Shiny app.
The two jobs differ in cost, process model, and persistence: the context index is synchronous, in-process, and in-memory (each session rebuilds its own), while pins warming is a background process filling a shared on-disk cache that can also be warmed offline ahead of deployment. Naming them separately makes call sites self-documenting and lets offline workflows warm only the persistent half. prewarm() remains as both. A persistent context store is noted as a possible future move (#214).
The store behind search_context was an in-memory ragnar store rebuilt from scratch by every process. It is now a DuckDB file keyed by a hash of the layer's docs (salted with ragnar/duckdb versions), so a build happens once per content version per cache root and every later session opens it read-only in milliseconds. Cold builds write a temp file and rename it into place atomically, so concurrent builders never expose a partial store; new content is a new key, so there is no invalidation logic. The cache root resolves from the commons.context_cache option, the COMMONS_CONTEXT_CACHE or CONNECT_CONTENT_DATA_DIR environment variables (Connect's early-access persistent data directories survive deployments), or the per-user cache dir. prewarm_context() now means 'ensure the store for this content exists' and can run offline, in CI, or at deploy time. Also close a race on the pins path: the background prewarm downloader and a first-use pin_read() could write the same cache entry concurrently (pins has no cache locking), risking a truncated entry that poisons later reads. Both sides now take an exclusive filelock keyed by cache path and pin name. Closes #214
Adopting the caching lessons from sass/bslib/shiny/cachem: - Cache root resolution is now context-aware (sass's convention): a hosted Shiny app uses app_cache/commons beside the app, scoping the cache per application on shared hosts; a local app uses it only if it already exists. - Stores unused for commons.context_cache_max_age seconds (default 30 days) are pruned, throttled cachem-style (once per 20 builds or 5s). Opens touch the mtime so age approximates LRU. Content-addressed immutable files make eviction safe: an evicted store still works for sessions holding it open, and the next opener rebuilds. - An unwritable cache dir warns once and falls back to a per-session tempdir (sass's graceful degradation) -- caching never breaks the app. - options(commons.context_cache = FALSE) disables persistence for dev loops, building the index in memory per layer. cachem itself was considered and rejected as a backend: cache_disk stores RDS values with no path API, and its any-process-can-evict semantics conflict with shared read-only opens of a DuckDB file.
rsconnect unconditionally excludes app_cache/ from deployed bundles (bundleFiles.R ignoreBundleFiles), so the app_cache cache root is per-deployment, not cross-deployment. Shipping a pre-built store with the app means pointing commons.context_cache at a bundle-included directory and running prewarm_context() before deploy.
Review-driven fixes on top of the persistent context store: - Validate options(commons.context_cache): a non-string, non-FALSE value now aborts instead of creating a directory named after the value. - Treat COMMONS_CONTEXT_CACHE=false/0/no (any case) as disabling the cache; env vars can't express FALSE, and a literal "FALSE" directory was previously created. - commons_prewarm() warms synchronously when no Shiny event loop is running (e.g. pre-deploy scripts), where a later::later() callback would never fire. - Wrap the prewarm span's cache_hit attribute in tryCatch so telemetry can never abort prewarming. - Replace age-based pruning with a single size-cap knob: options(commons.context_cache_max_size) (default 256 MB) with LRU eviction by mtime (touched on open). The just-built store is explicitly protected, and a single store larger than the cap is kept with a one-time warning, matching cachem's behavior, rather than evicted into a rebuild loop. - Fix the persistent-store test to read docs via context_layer_state(); layer$docs returns NULL now that layer internals are private. - Credit the sass package's file cache as prior art in the commons_prewarm() docs.
… stores - prune_context_cache() reaps .build-* temp files older than 24h so crashed builds can't leak partial stores outside the size cap, and only decrements the size total when an eviction unlink succeeds. - context_store() warns (and notifies in Shiny) and rebuilds once when the cached store fails to open, e.g. unlinked by a concurrent pruner; a second failure still propagates. - Note the pins-version assumption in with_pin_lock() and clarify in ?commons_prewarm that failures are downgraded even on the synchronous path.
|
Preview deployed to Connect ( Deployed from commit 672fd59. |
|
Preview deployed to Connect ( Deployed from commit 672fd59. |
- commons_prewarm(): interpolate the error message safely so braces in raw error text (e.g. DuckDB's embedded JSON) can't throw inside the handler and escape the later::later() callback. - context_cache_dir_safe(): probe writability by creating and deleting a temp file instead of file.access(), which checks DOS attributes rather than ACLs on Windows. - reap_stale_build_files(): drop NA mtimes from files deleted by a concurrent process mid-call. - Document with_pin_lock()'s lock-name collision and lock-file litter.
simonpcouch
left a comment
simonpcouch
left a comment
There was a problem hiding this comment.
Could you do a pass on the code comments (incl the roxygen) and also write a couple sentences in your own words about what this PR does and what I should pay attention to?
On first glance, it seems like this exports a new function in the namespace instead of making slots on the data source / context layer public. Open to this idea, but any reason why? It also seems like we've introduced a number of user-facing envvars and options. Should any be function arguments?
- prune_context_cache() drops stores that stat as NA (deleted by a concurrent pruner mid-prune) instead of erroring on NA > max_size, and skips eviction victims that vanish before unlink. - context_cache_dir_safe() suppresses the file.create() probe warning so only the once-per-session cli warning surfaces. - Extract context_store_dir() so the cache's context/ layout has a single source of truth.
commons_prewarm() is now synchronous-only, for warming caches from a script ahead of deployment: it takes a cache_dir argument (scoped to the call) so the warmed context index can ship inside the app bundle, and announces where the cache landed with guidance on making it reachable from a deployment (including that rsconnect excludes app_cache/ from bundles). The Shiny idle-time path moved to an internal prewarm_on_idle() helper that owns the later::later() deferral and warning downgrade. The agent's public prewarm surface is now just prewarm(); prewarm_context() and prewarm_sources() are private. Docs rewritten for new users: what prewarming does, when commons_server() already handles it, when to configure the cache directory (ephemeral hosts like Connect Cloud, shipping a warm cache, dev loops), and the early-access status of Connect's persistent data directories.
The core idea is a persistent cache for the ragnar store, which should help with loading times, especially for larger stores. With the current in-memory approach, the cache is lost every time the server goes to sleep (i.e., after ~10 mins of inactivity), and does nothing for deployments that dedicate one process per visitor. Instead, this PR takes a file-based cache approach (works since Ragnar stores are backed by DuckDB files), which allows the cache to persist even across deploys is some situations. As a result, this should reduce the need for workarounds like https://github.com/posit-dev/tiles-agent/pull/37 to improve startup time.
Much of the implementation here is borrowed from
I believe this is in reference to
The options this PR adds is |
2 participants
What changes
Building the ragnar context index (DuckDB + FTS) was previously the most expensive part of constructing a
commons()agent, and every session paid for it: the store lived in memory, scoped to the layer object, so a new process meant a full rebuild. This PR makes the store a persistent, content-addressed file on disk and adds explicit prewarming hooks so the cost can be moved off the critical path — or off the deployed app entirely.The key ideas
Content-addressed, not session-addressed. The store's filename hashes
the layer's documents plus the ragnar/duckdb package versions (whose on-disk format the store depends on). Correct invalidation falls out for free: edit a doc or upgrade a dependency and you get a new file; change nothing and every session, process, and deployment opens the same one. There's no cache key management to get wrong because the content is the key.
Build once, then everyone just opens a file. Builds go to a temp file in
the same directory and are renamed into place atomically, so no reader ever observes a partial store, and two concurrent builders degrade to "loser discards an equivalent store." Readers connect read-only (
ragnar_store_connect()'s default), so concurrent Shiny sessions across processes can share one store file safely. The cache is size-capped (256 MB by default) and pruned LRU, with an mtime touch on open keeping actively used stores young. Temp files from interrupted builds (killed processes, crashes) are reaped once they're a day old, so debris can't accumulate outside the size cap.The cache root is deployment-aware. Resolution order: an explicit
option/env override, then Connect's persistent content data dir (
CONNECT_CONTENT_DATA_DIR, survives redeploys where enabled), then anapp_cache/commons/directory beside the app (the sass convention: per-app scratch on hosted platforms, excluded from deployed bundles), then the per-user cache dir. On ephemeral filesystems (Connect Cloud resets disk to the bundle), the store simply rebuilds once per cache lifetime instead of once per process. And because the store is just a file, you can ship a warm cache with the app: pointcommons.context_cacheat a directory inside the app and runprewarm_context()before deploying. An unwritable cache root warns once and falls back to a per-session tempdir — caching can never take down the app.Prewarming is explicit and failure-semantics-aware.
agent$prewarm()splits into
prewarm_context()(build the index) andprewarm_sources()(background pin download). A direct call lets failures propagate — warming ahead of a deploy should fail the deploy.commons_prewarm()is the Shiny path: it defers warming to alater::later()callback and downgrades failures to warnings, since an escaping error would stop the app and prewarming is a pure optimization. Outside a running Shiny app (e.g. a pre-deploy script) there's no event loop, so it warms synchronously.How this relates to the old design
The previous
commons_context_layerwas a plain list carrying acacheenvironment, andcontext_store()memoized an in-memory (:memory:) ragnar store inlayer$cache$storeon first search. The environment was the whole trick: reference semantics meant every copy of the layer shared one store, and becauseaugment_context_layer()built a fresh object (with a fresh, empty environment) whenever docs changed, a stale store could never be reused. In effect it was lazy memoization keyed by object identity — which meant the store died with the R process, and "sharing" only worked if you literally passed the same object around. Every new session, Connect worker, or redeploy rebuilt the index from scratch.This PR keeps that design's shape — lazy build, cached on the layer, fresh store when docs change — but changes what the cache is:
"same docs + same ragnar/duckdb versions → same file on disk." Sharing no longer requires passing one object around; independently constructed layers with identical docs hit the same cache entry, across processes and deployments.
:memory:→ persistent file. The expensive build happens once percontent version per cache root instead of once per session.
ContextLayer's privatestorefield plays the old environment's in-process role (repeat searches in a session don't reopen the file); the disk cache sits underneath it as the cross-process layer. The fresh-store-on-doc-change property is preserved twice over: augmenting docs still creates a new layer object, and different docs hash to a different filename anyway.The old code paths are fully removed —
new_context_layer()no longer creates the list-plus-environment structure, and nothing referenceslayer$cacheanymore.What this means in practice
of a full chunk + index build.
commons_server()prewarms automatically during post-startup idle time,while the user reads the welcome message.
Prior art
Much of the logic and motivation here is adapted from the sass R package, whose file cache (
sass_cache_*()) has run in production Shiny deployments for years. The pieces we borrow directly: the cache-root resolution ladder (explicit option → env var →app_cache/beside the app on hosted platforms → per-user cache dir), content-addressed keys so invalidation is structural rather than managed, size-capped LRU eviction, and the posture that caching is a pure optimization that must never take down the app. sass's cache has survived every hosting quirk Connect and Shiny Server have thrown at it; following its precedent is deliberate risk reduction, not novelty.Escape hatches
options(commons.context_cache = FALSE)(orCOMMONS_CONTEXT_CACHE=false)disables persistence — stores build in memory per layer, as before.
options(commons.context_cache_max_size)caps total cache size (default256 MB) with least-recently-used eviction — the single pruning knob. Following cachem's precedent, a single store larger than the cap is kept (with a one-time warning) rather than evicted into a rebuild loop.
Also in this PR
commons_data_source,commons_semantic_layer,commons_context_layer) become R6 classes with private state, so connections, processes, and caches have explicit ownership.display.
non-
FALSEvalue aborts rather than creating a directory named"TRUE"), and telemetry span attributes can never abort prewarming. A store that fails to open (e.g. unlinked by a concurrent pruner between the existence check and the connect) warns — with a Shiny notification when running in an app — and is rebuilt once; a second failure still surfaces.