Skip to content

feat(cache): bound query cache TTL on shared Redis - #6905

Open
zuoanCo wants to merge 5 commits into
volcengine:mainfrom
zuoanCo:feat/cache-admission-priority
Open

zuoanCo wants to merge 5 commits into
volcengine:mainfrom
zuoanCo:feat/cache-admission-priority

Conversation

@zuoanCo

@zuoanCo zuoanCo commented Nov 2, 2026 •

Copy link
Copy Markdown

Description

Follow-up to @baojun-zhang's review on #6903. After #6346, CacheFS stores query results rather than file bodies, so the old .abstract.md / .overview.md / directory-entry priority model no longer maps to cached objects.

This revision drops that obsolete mapping and scopes the config to the shared-backend capacity concern:

  • Renames the option to storage.agfs.cachefs.shared_redis_ttl_seconds.
  • Applies the override uniformly to all query buckets (stat, read_dir, tree_directory, glob_directory, grep).
  • Activates it only when CacheFS shares Redis with QueueFS, a cache-backed PathLock, or the expiry scheduler.
  • Keeps isolated Redis behavior unchanged (0 uses the provider default).
  • Documents that TTL is a retention bound, not hard capacity isolation. A dedicated Redis service is required for a strict capacity boundary; changing a logical database does not isolate maxmemory or eviction policy.

Human Involvement

  • A human participated in the implementation or review loop
  • This PR was generated entirely by AI agents without human participation in the loop

Related Issue

Fixes #6903

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)

Changes Made

  • crates/ragfs/src/cache/wrapper.rs: applies the shared-Redis TTL uniformly to query buckets and removes file-level priority matching.
  • crates/ragfs/src/core/builder.rs, mountable.rs, and crates/ragfs-python/src/lib.rs: propagate the renamed config through the query-driven stack.
  • openviking/utils/agfs_utils.py: enables the override only when the cache shares a Redis backend.
  • openviking_cli/utils/config/agfs_config.py: validates cachefs.shared_redis_ttl_seconds.
  • Docs: describe query-bucket semantics and shared-Redis deployment limits.
  • Tests: cover shared/isolated activation and uniform query bucket TTL selection.

Testing

python -m pytest -q tests/misc/test_config_validation.py -k "shared_redis_ttl or shared_redis_backend or canonical_cache_schema" --no-cov
# 3 passed, 72 deselected

python -m pytest -q tests/misc/test_config_validation.py --no-cov
# 74 passed, 1 known Windows-only path expectation failure:
# test_generate_plugin_config_materializes_queuefs_paths[None-None-expected3]
# compares '/tmp/...' with '\tmp\...'

python -m ruff check openviking/utils/agfs_utils.py openviking_cli/utils/config/agfs_config.py tests/misc/test_config_validation.py
# All checks passed

Rust compilation could not run on this Windows host because the MSVC linker (link.exe) is unavailable. The affected Rust blocks were checked with rustfmt --check; the repository-wide cargo fmt --check already reports unrelated pre-existing formatting differences on main.

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented on hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings (Rust compilation requires CI on this host)
  • Any dependent changes have been merged and published

CacheDecision::Prefer is defined in crates/ragfs/src/cache/policy.rs and
marks high-value objects (.abstract.md, .overview.md, directory entries),
but no layer ever consumed it: the cache wrapper only asked the policy for
a boolean, so a preferred entry was cached exactly like any other one.

Give preferred entries their own TTL through storage.agfs.cachefs
.priority_ttl_seconds. The wrapper now passes the admission decision down
to the provider write and sets SetOptions.expiration for preferred entries,
so a provider that evicts by TTL keeps them longest.

The TTL only applies when the cache competes with stateful keys for the
same Redis service: QueueFS and PathLock reuse the top-level cache
provider, and the expiry scheduler only shares the backend when both
configurations point at the same endpoints. With separate Redis services
there is nothing to yield to, so the option stays disabled.

priority_ttl_seconds defaults to 0, which keeps the current behavior.
@zuoanCo

zuoanCo commented Nov 2, 2026

Copy link
Copy Markdown
Author

@baojun-zhang this touches the Storage area (crates/ragfs, openviking/storage), so routing it to you.

Summary of the change:

  • CacheDecision::Prefer in crates/ragfs/src/cache/policy.rs had no consumer. CachedFileSystem only asked the policy for a boolean, so .abstract.md, .overview.md, and directory entries were cached exactly like ordinary body files — even though docs/*/guides/14-ragfs-cache.md already states that the policy prefers those summary files.
  • The wrapper now passes the admission decision down to the provider write and applies storage.agfs.cachefs.priority_ttl_seconds to preferred entries.
  • The option only takes effect when the cache shares a Redis service with QueueFS, PathLock, or the expiry scheduler. QueueFS and a cache-backed PathLock reuse the top-level cache provider, so they always share it; the expiry scheduler is compared by endpoint. With separate Redis services there is no shared capacity to yield to, so the option stays disabled.
  • priority_ttl_seconds defaults to 0, which keeps the current behavior unchanged.

Validation: cargo test -p ragfs --lib (493 passed), cargo test -p ov_cli --bin ov (528 passed, needs RUST_MIN_STACK=33554432 — the help_ui stack overflow reproduces on unmodified main as well), cargo check -p ragfs-python, ruff, and mypy with no new findings.

One note: this is a fork PR, so the workflows are sitting at action_required and need a maintainer to approve them before CI runs.

There is one open question in the issue I would like your view on: on a shared backend, should the same "yield to stateful keys" reasoning also shrink max_file_size_bytes, or is the TTL split enough?

@ZaynJarvis ZaynJarvis added the kernel Core server, runtime, storage, retrieval, SDK, and CLI. label Nov 3, 2026
Adapt prioritized cache TTLs to the query-driven CacheFS runtime introduced on main.
Adapt the shared Redis design to the query-driven CacheFS model. Apply the configured TTL uniformly to query buckets instead of preserving obsolete file-level priority semantics, and document that TTL is not a hard capacity-isolation boundary.
@zuoanCo zuoanCo changed the title feat(cache): honor cache admission priority on shared Redis backends feat(cache): bound query cache TTL on shared Redis Nov 4, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kernel Core server, runtime, storage, retrieval, SDK, and CLI.

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

[Feature]: Cache admission priority for shared Redis backends

2 participants