Skip to content

[Feature]: Cache admission priority for shared Redis backends #6903

Open

Description

@zuoanCo

Issue Origin

Observed or reproduced in a real environment

Problem Statement

CacheDecision::Prefer is defined in crates/ragfs/src/cache/policy.rs and marks high-value objects (.abstract.md, .overview.md, and directory entries), but no layer ever consumes it. CachedFileSystem only asks the policy for a boolean (cache_file() / cache_directory()), so a preferred entry is cached exactly like any other one. The docs already describe the intended behavior ("The default policy prefers summary files such as .abstract.md and .overview.md"), but the implementation draws no distinction between a summary and an ordinary body file.

The gap matters when CacheFS shares a Redis service with stateful modules. QueueFS (storage.agfs.queuefs.backend = "cache") and a cache-backed PathLock (storage.agfs.pathlock.provider = "cache") reuse the top-level cache provider, and the expiry scheduler can be pointed at the same Redis service through expiry.redis. In that layout every module competes for one maxmemory budget and one eviction policy, and there is no way to say that a summary should outlive an ordinary body file, or that cache writes should yield to the stateful keys.

Observed on a production deployment that serves cache, QueueFS, PathLock, and the expiry scheduler from a single Redis service: the cache writes far more than the TTL expires (29,802,144 LRU evictions against 258,954 TTL expirations over five days), so the eviction policy, not the cache policy, decides what survives.

Proposed Solution

Add storage.agfs.cachefs.priority_ttl_seconds and let the cache wrapper apply it to preferred entries.

  • The wrapper 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 option only applies when the cache actually shares a Redis service with QueueFS, PathLock, or the expiry scheduler. QueueFS and PathLock reuse the top-level cache provider, so they always share it; the expiry scheduler owns its own connection and only shares the backend when both configurations point at the same endpoints. 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 byte for byte.

A pull request implementing this is open at #6905.

Alternatives Considered

  • Separate Redis services for cache and stateful modules. This removes the contention entirely, but it doubles the operational surface and is unnecessary for small deployments. It also does not solve the in-cache priority question, which stays open.
  • A real per-key priority in the cache provider. Redis has no per-key priority, so a provider-level implementation would need a separate eviction domain per priority class, which means separate databases or separate instances. The TTL approach keeps the change inside the existing SetOptions contract and works with any provider that supports expiration.
  • Excluding body files from the cache entirely. bypass_prefixes is prefix-based and cannot express "skip large body files but keep their summaries", so this would drop the summaries too.

Feature Area

Storage/VectorDB

Use Case

A self-hosted deployment that runs a single Redis service for the whole RAGFS stack: file and directory cache, the add-resource and semantic queues, path locks, and the expiry scheduler for scheduled compiles. Cache writes are high volume and bursty, so the cache needs a way to keep the objects that retrieval reads most often (the L0 abstracts and L1 overviews) without giving up the whole cache.

Example API (Optional)

{
  "storage": {
    "agfs": {
      "cachefs": {
        "backend": "cache",
        "max_file_size_bytes": 262144,
        "priority_ttl_seconds": 7200
      },
      "queuefs": { "backend": "cache" },
      "pathlock": { "provider": "cache", "lock_expire_secs": 30.0 }
    }
  },
  "compile_api": { "scheduler_enabled": true, "scheduler_backend": "redis" },
  "expiry": {
    "service_name": "openviking-server",
    "redis": {
      "mode": "standalone",
      "endpoints": ["redis://openviking-redis:6379"],
      "username": "ov-expiry",
      "password_env": "OPENVIKING_EXPIRY_REDIS_PASSWORD"
    }
  }
}

Additional Context

The shared-service detection lives in openviking/utils/agfs_utils.py, because that layer sees both the top-level cache provider and the expiry block. The RAGFS binding itself only receives the cache configuration and cannot compare the two endpoints.

Open question for maintainers: should the same "yield to stateful keys" reasoning also apply to max_file_size_bytes (for example, a smaller admission limit when the backend is shared), or is the TTL split enough?

Contribution

  • I am willing to contribute to implementing this feature

Activity

  1. moved this from Backlog to In progress in OpenViking projecton Nov 2, 2026
  2. zuoanCo commented on Nov 2, 2026

    @zuoanCo
    Author

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

    The core finding is that CacheDecision::Prefer is currently dead code: it marks .abstract.md, .overview.md, and directory entries as high-value, but CachedFileSystem only asks the cache policy for a boolean, so a preferred entry is cached exactly like an ordinary body file. docs/*/guides/14-ragfs-cache.md already documents the preferred behavior, so this is a documentation/implementation mismatch rather than a missing feature.

    The proposed storage.agfs.cachefs.priority_ttl_seconds gives preferred entries their own TTL, and only applies when the cache actually 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.

    An implementation PR is open for review: #6905. Please treat this issue as the design discussion and the PR as the concrete proposal.

    The open question I would appreciate input on: on a shared Redis backend, should the "yield to stateful keys" reasoning also apply to the cache admission limit (max_file_size_bytes), or is separating the TTL enough?

  3. baojun-zhang commented on Nov 3, 2026

    @baojun-zhang
    Collaborator

    Thanks @zuoanCo for the detailed analysis and for contributing the implementation proposal in #6905.

    The issue is correct for the pre-refactor CacheFS model. The old implementation had CacheDecision::Prefer to mark .abstract.md, .overview.md, and directory entries as high-value cache objects, but CachedFileSystem ultimately consumed only the boolean admission result. In practice, Prefer did not change the write TTL or eviction behavior, so the documentation and implementation did not match.

    However, the CacheFS refactor in #6346 changes the caching model, so this problem no longer exists in the same form.

    The new design no longer caches file bodies, and it no longer uses CachePolicy, CacheDecision, or max_file_size_bytes as the admission policy. CacheFS is now query-driven:

    • Cache objects are query results, not file contents or raw directory entries.
    • Each path + operation maps to one Redis HASH bucket.
    • Different request parameter sets are stored in the same bucket by query hash.
    • Cached payloads include the request envelope, and hits must match the request exactly.
    • Writes, deletes, renames, and related mutations invalidate affected query buckets.
    • Cache fill uses a bucket epoch with CAS validation, so an old query cannot refill stale data after invalidation.

    Because of that, the old distinction between .abstract.md / .overview.md and ordinary body files no longer has a corresponding cached object in the new model. CacheDecision::Prefer has also been removed, so there is no longer a dead code path where Prefer is defined but not consumed.

    The shared Redis capacity concern is still worth discussing, but it should be revisited under the new query-cache model. The question is no longer whether summary files should get a longer TTL than ordinary files. It is whether query buckets sharing Redis with QueueFS, PathLock, or other stateful keys need additional capacity isolation, TTL policy, or deployment guidance.

    My suggestion is that #6905 should not be merged directly on top of the old CacheFS model. If we still need to address shared Redis capacity pressure, the design should be updated based on the query-driven CacheFS model introduced in #6346.

  4. added
    kernelCore server, runtime, storage, retrieval, SDK, and CLI.
    on Nov 3, 2026
  5. zuoanCo commented on Nov 4, 2026

    @zuoanCo
    Author

    Thanks @baojun-zhang, I updated #6905 based on your review.

    The PR no longer maps the old .abstract.md / .overview.md / directory-entry priority model onto query buckets. After #6346, the option is now storage.agfs.cachefs.shared_redis_ttl_seconds and applies uniformly to all query buckets only when CacheFS shares Redis with QueueFS, a cache-backed PathLock, or the expiry scheduler. Isolated Redis behavior remains unchanged.

    The docs now explicitly state that this is a cache-retention bound, not hard capacity isolation: a strict boundary requires a dedicated Redis service, and changing a logical database does not isolate maxmemory or eviction policy. Please take another look when you have a chance.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    kernelCore server, runtime, storage, retrieval, SDK, and CLI.

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions