perf: SDK read cache + in-flight dedup; FUSE content TTL 2s→30s (issue #300 solutions A+B)#303
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Review limit reached
More reviews will be available in 39 minutes and 32 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a client-side ChangesTypeScript SDK: FileReadCache
Go FUSE: ContentTTL Decoupling
Sequence Diagram(s)sequenceDiagram
participant Caller
participant RelayFileClient
participant FileReadCache
participant SyncStream
participant Server
Caller->>RelayFileClient: readFile(workspaceId, path)
RelayFileClient->>FileReadCache: get("workspaceId:path")
alt TTL cache hit
FileReadCache-->>RelayFileClient: cached content
RelayFileClient-->>Caller: content (no HTTP)
else in-flight dedup
FileReadCache-->>RelayFileClient: existing Promise
RelayFileClient-->>Caller: shared Promise
else cache miss
RelayFileClient->>Server: GET /fs/file
Server-->>RelayFileClient: file content
RelayFileClient->>FileReadCache: set(key, content, TTL)
RelayFileClient-->>Caller: content
end
SyncStream-->>RelayFileClient: file.updated / file.created / file.deleted
RelayFileClient->>FileReadCache: evict("workspaceId:path")
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 018c5702dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Transient HTTP error (503, 429, etc.): skip this path for | ||
| // the current cycle. The cursor-based resume on the next | ||
| // cycle will retry it without restarting from scratch. | ||
| s.logf("transient error reading %s (status %d); skipping for this cycle: %v", result.RemotePath, httpErr.StatusCode, result.Err) | ||
| continue |
There was a problem hiding this comment.
Do not complete bootstrap after transient read failures
When ListTree succeeds but a per-file ReadFile returns a non-403 HTTP error such as 429 or 503, this branch logs and continues without adding result.RemotePath to remotePaths or preserving a retry cursor for that file. If the traversal started from an empty cursor, the code later treats the snapshot as authoritative, runs snapshot deletes, and marks bootstrap complete, so an existing local copy of the skipped path can be deleted and the next cycle will not retry it. Propagate retryable HTTP errors, or otherwise keep the path out of the authoritative delete/complete path, rather than treating them like denied reads.
Useful? React with 👍 / 👎.
| if (cache && cacheKey) { | ||
| const hit = cache.get(cacheKey); | ||
| if (hit) return hit; | ||
| const pending = cache.getInFlight(cacheKey); | ||
| if (pending) return pending; |
There was a problem hiding this comment.
Evict cached reads when SDK writes mutate paths
With readCache enabled by default, this cache-hit path can return a cached FileReadResponse even after the same RelayFileClient has had a writeFile, bulkWrite, or deleteFile request accepted for that workspace/path, because those mutation methods do not evict fileReadCaches unless an active change stream later delivers an event. In read→write/update/delete→read flows without an active stream, callers can keep seeing stale content for ttlMs (5s by default) instead of the state the server would now return; evict the affected keys when mutations are issued or accepted.
Useful? React with 👍 / 👎.
Solutions A and B from issue #300: SDK FileReadCache (client.ts): - In-process LRU cache keyed by workspaceId:path, default 5s TTL, max 500 entries. In-flight deduplication: concurrent reads for the same key share one outstanding Promise instead of issuing parallel requests. - Active change streams evict cache entries immediately on file.updated / file.created / file.deleted, so TTL expiry is only a fallback. - Fork-scoped reads (forkId set) bypass the cache — fork state is isolated. - Opt-out: pass readCache: false to RelayFileClientOptions. - New type RelayFileReadCacheOptions exported from @relayfile/sdk. FUSE content TTL (fs.go, main.go, fuse_mount.go): - Config.ContentTTL decoupled from AttrTTL. Default 30s vs previous 2s. AttrTTL (kernel attribute cache) stays at 2s for fast stat; only the in-memory content blob is extended. - WSInvalidator already evicts on remote change so longer TTL does not increase staleness risk. - Configurable via --fuse-content-ttl flag or RELAYFILE_MOUNT_FUSE_CONTENT_TTL env var for operator tuning. Live prod impact (wrangler tail sample, 89 events / 43s): - 8 of 11 file reads are /discovery/** schema re-reads → SDK cache collapses repeated reads to 1 per TTL window. - Go mount client = 55/89 events → FUSE TTL 2s→30s reduces re-fetches ~15x. - Combined projected reduction: ~3-4x on fs/file volume. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
018c570 to
bfa2957
Compare
Root and standalone SDK lockfiles both had mount binary packages pinned at 0.9.6 while package.json requires 0.10.0, causing `npm ci` to fail in Contract CI. Updated all four platform binaries in both lockfiles with registry-verified integrity hashes for 0.10.0. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Relayfile Eval ReviewRun: Passed: 4 | Needs human: 0 | Reviewable: 0 | Missing output: 0 | Failed: 0 | Skipped: 0 Human Review CasesNo reviewable human-review cases captured Relayfile output. |
Without this, a read→write→read sequence on the same client without an active change stream could return stale cached content for up to ttlMs (default 5s). Now cache entries for mutated paths are evicted immediately when the server accepts the write, matching the behaviour callers expect. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Addresses solutions A and B from issue #300 (high service call volume when agents read/write through the deployed API). No server-side changes required.
Live prod signal (wrangler tail, 89 events / 43s): 8 of 11 file reads are
/discovery/**schema re-reads; Go mount client accounts for 55/89 events. Both patterns are directly cut by these changes.Solution A — SDK read cache with in-flight deduplication
FileReadCacheclass added toclient.ts, stored per-client viaWeakMap<RelayFileClient, FileReadCache>(same pattern aschangeLogSettings){workspaceId}:{path}share one outstandingPromise— no parallel duplicate requestssubscribe/open) evict the cache entry immediately onfile.updated,file.created,file.deleted— TTL is only a fallback for paths with no active subscriptionforkId-scoped reads skip the cache (fork state is isolated)readCache: falseinRelayFileClientOptionsRelayFileReadCacheOptionsfrom@relayfile/sdkSolution B — FUSE content cache TTL decoupled from kernel attrTTL
Config.ContentTTLadded tomountfuse.Config(default 30s, previously shared 2sattrTTL)AttrTTL(kernel attribute cache) stays at 2s —stat()still refreshes quicklyWSInvalidatoralready evicts on remote change--fuse-content-ttlflag orRELAYFILE_MOUNT_FUSE_CONTENT_TTLenv varImpact projection
readFilecallsFrom wrangler tail data: combined projected ~3–4× reduction on
fs/filevolume.Drift note
RelayFileReadCacheOptionsis now the canonical SDK type. Cloud should consume via@relayfile/sdk— not re-implement caching locally. A follow-up cloud PR (solutions C+D: bulk-read + ETag/304) is in progress separately.Test plan
npx tsc --noEmit— cleanvitest run)go build ./internal/mountfuse/... ./cmd/relayfile-mount/...— cleango test ./internal/mountfuse/...— pass--fuseand--fuse-content-ttl=60s, confirm content served from cache between readsCloses #300 (solutions A and B).
🤖 Generated with Claude Code