Skip to content

feat(csc): multi-strategy client-side caching (Broadcast / PerConnection / SharedTracking)#3851

Open
ofekshenawa wants to merge 5 commits into
csc-standalone-connection-supportfrom
csc-multi-strategy
Open

feat(csc): multi-strategy client-side caching (Broadcast / PerConnection / SharedTracking)#3851
ofekshenawa wants to merge 5 commits into
csc-standalone-connection-supportfrom
csc-multi-strategy

Conversation

@ofekshenawa

@ofekshenawa ofekshenawa commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator

Note

Medium Risk
Large correctness-sensitive change to caching, invalidation timing, and connection lifecycle; mitigated by extensive tests and conservative flush/disable paths on sidecar failure and disconnect.

Overview
Adds Options.ClientSideCacheStrategy (CSCStrategySharedTracking default, Broadcast, PerConnection) so RESP3 client-side caching can pick how invalidations reach the cache, with docs/csc-strategy-guide.md describing trade-offs.

SharedTracking (default behavior change): drops the per–cache-hit drainPendingInvalidations sweep and runs a background drainer (configurable DrainInterval, default 5ms) that uses pool.DrainIdleConns and drainPushNotifications with a hard read deadline. Cache hits are served without re-draining the pool first.

Broadcast: a dedicated BCAST sidecar (CLIENT TRACKING ON BCAST) feeds the shared cache; pool connections do not enable tracking. Sidecar disconnect/reconnect flushes the cache; failed sidecar start disables CSC for that client.

PerConnection: private localCache per pool conn, commands bound to one conn via cscPerConnTryProcess, with invalidations routed to that conn’s cache only.

localCache: sharded storage with approximate LRU, plus MaxStaleness and DrainInterval on CacheConfig. CSC is limited to DB == 0 (logged disable otherwise).

Observability: Client.CSCStats(), process-wide CommandStats() / RESPInvalidationBytesRead(), and pool conn lastPushDrainAt to skip redundant push peeks.

Reviewed by Cursor Bugbot for commit 6f3ca29. Bugbot is set up for automated code reviews on this repo. Configure here.

@jit-ci

jit-ci Bot commented Jun 14, 2026

Copy link
Copy Markdown

🛡️ Jit Security Scan Results

CRITICAL HIGH MEDIUM

✅ No security findings were detected in this PR


Security scan by Jit

@ofekshenawa ofekshenawa marked this pull request as ready for review June 29, 2026 08:07

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 6f3ca29. Configure here.

Comment thread redis.go
// Socket peek confirmed no pending data — stamp the conn so a tight
// follow-up op (the CSCStrategyPerConnection hot path) can skip the
// redundant syscall.
cn.SetLastPushDrainAtNs(pool.GetCachedTimeNs())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reader-buffered pushes skipped

Medium Severity

Sharded NewLocalCache assigns each shard ceil(MaxEntries / shardCount) as its own cap and evicts per shard independently, so total resident entries can exceed CacheConfig.MaxEntries (e.g. 100 configured with 16 shards allows up to 112).

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6f3ca29. Configure here.

Comment thread csc_integration.go
if data, ok := c.csc.Get(ctx, key); ok {
if err := applyCachedReply(cmd, data); err == nil {
return nil
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SharedTracking drainer skips custom pools

Medium Severity

startBackgroundDrainer no-ops unless connPool is *pool.ConnPool, and processCached no longer drains invalidations on cache hits. SharedTracking clients with a custom Pooler may apply idle invalidations only when a connection runs a command, widening staleness beyond the documented drain interval.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6f3ca29. Configure here.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant