1
0
mirror of https://github.com/redis/go-redis.git synced 2026-10-24 07:59:20 +03:00
Files
Nedyalko Dyakov f6ffe09a91 feat(csc): Refresh and miss coalescing (#3989)
* feat(csc): refresh-on-invalidate and reader-miss coalescing

Two additions over the shared-tracking client-side cache, both inert
unless CSC is enabled.

Refresh-on-invalidate re-fetches recently-read keys as soon as their
invalidation push arrives, instead of leaving them to be reloaded by the
next reader that misses. Invalidated cache keys that were valid and read
within the recency horizon are collected under the shard lock as they are
deleted, batched over a fixed window from the first collect, and flushed
when the window expires, when a size cap is hit, or on demand the moment a
reader misses a key still sitting in the window. Refetches reserve each
cache key before touching the network and release any reservation they do
not publish, so a reader never blocks on an orphaned in-progress entry.
Reads run on a background pipelined connection. Enabled via
Options.ClientSideCacheRefreshOnInvalidate; window and demand behaviour
tunable through GOREDIS_CSC_REFRESH_WINDOW_MS and GOREDIS_CSC_REFRESH_DEMAND.

Reader-miss coalescing pipelines concurrent cache-miss reads onto a single
CSC-tracked main-pool connection. Each miss is sent as the caller's own
per-key command (not rewritten to MGET, so it is cluster-safe); every reply
is applied to its caller's command and published to the cache under the
connection's captured tracking generation, so entries stay invalidatable.
initConn issues CLIENT TRACKING ON for every pooled connection while CSC is
active, so the main pool is tracked; using it explicitly (rather than the
pipeline pool) keeps the path correct even if a later change makes a
dedicated pipeline pool deliberately untracked, since publishing on an
untracked connection would serve stale until TTL. Enabled via
GOREDIS_CSC_COALESCE_MISSES; worker count via GOREDIS_CSC_COALESCE_WORKERS
(default 8).

Adds LocalCache.deleteByRedisKeyCollectingHot / LRUClock / InvalidationStats
and per-cache invalidation counters to support the above, with unit coverage
of the recency filter and an integration test asserting a refreshed entry is
itself invalidatable.

* feat(csc): full-duplex reader-miss coalescing + invalidation batching (#3965)

* docs: point security reports at Redis VDP (#3949)

* fix(pool): log buffered push data at debug level (#3948)

The buffered-push-data notice in isHealthyConn fired unconditionally.
  With client-side caching every tracked write parks an invalidate frame
  on idle pooled conns, so at default log level the line floods the log
  on nearly every pool Get under write-heavy workloads. It documents the
  normal healthy path, not a problem - gate it behind LogLevelDebug like
  the equivalent notices in maintnotifications.

* feat(csc): reader-miss coalescing modes (workers + full-duplex) and invalidation batching

Builds on the CSC refresh-on-invalidate + base miss-coalescing PR. Adds:

- Coalescing MODES for the reader-miss path, selected by config:
  * "workers" (default): a small pinned worker pool fetches reserved misses.
  * "fullduplex": one held connection with a writer + reader goroutine pair
    pipelines the reserved misses (concurrent, replies streamed back). Ordering
    is preserved: the miss-coalescer's reservation dedups concurrent misses of
    the same key to a single fetch, and each caller blocks on its own request's
    completion — the caching client is blocking per goroutine, so no goroutine
    ever sees its own reads reordered; full-duplex only overlaps independent
    goroutines' fetches on the wire.
- Invalidation batching: coalesce invalidation-driven cache deletes within a
  configurable window instead of one delete per push.

Config (no environment variables): new AutoPipeline-free Options fields —
ClientSideCacheCoalesceMisses (enable), ClientSideCacheCoalesceMode
("workers"/"fullduplex"), ClientSideCacheCoalesceWorkers, and
ClientSideCacheInvalidationBatchWindow. Prototype-only read-path telemetry
(READPATH_LOG + LocalCache stat counters) removed; no stats surface added.

Measured on a 50ms-RTT WAN proxy under invalidation churn, the coalescing fixes
turn published v9.22.0's miss-stampede collapse (p99 up to ~1.1s, throughput
floored) into ~5x throughput at ~1 RTT p99, matching rueidis; the full-duplex
mode gives the tighter mid-range tail at fewer connections. See
AP_CSC_TWOCLIENT_VS_RUEIDIS.md.

Depends on: feature/csc-refresh-and-miss-coalescing.

* fix(csc): address #3965 review and lint in coalescing modes

- lint (the only red CI check): drop the redundant `chan struct{}` type from the
  recycle declaration (ST1023) and //nolint:unused the deliberately-off, measured
  cscRefreshCooldown knob.
- Reject the "pinned" PROTOTYPE engine from the public ClientSideCacheCoalesceMode
  option: it holds a connection with no idle invalidation drain and can serve
  stale values (cursor HIGH). It now falls back to "workers" and is reachable
  only via an internal benchmark hook (cscForcePinned).
- Pass the new CSC miss-coalescing / invalidation-batching knobs through
  UniversalOptions.Simple() (ClientSideCacheRefreshOnInvalidate, CoalesceMisses,
  CoalesceMode, CoalesceWorkers, InvalidationBatchWindow) so UniversalClient
  users can enable them (codex P2).
- Give the invalidation batcher a stop path: it is stopped and cleared when the
  last user releases the binding (releaseLocked), and ensureBatcher refuses to
  start one for an already-released binding, so its goroutine no longer lives
  past the binding re-arming its timer forever; a later re-acquire starts fresh.
- Full-duplex session: acquire the pool connection only after the first miss
  arrives, and release it if CSC serving was disabled meanwhile, so an idle
  session no longer starves a small pool (PoolSize:1) until PoolTimeout.

Adds unit tests for the pinned-mode rejection and the UniversalOptions passthrough.

* fix(csc): FD apply-and-gate on id/gen mismatch; drain buffered pushes; doc

- Full-duplex readOne now applies the read reply and lets fulfillCached gate
  only the cache publish on the captured conn id/generation -- matching the
  workers/pinned engines -- instead of failing the caller with ErrClosed and
  losing a good reply on a mid-flight id/gen change.
- peekAndProcessPushNotifications also drains when the reader has buffered bytes
  (HasBufferedData), not only when the socket is readable (MaybeHasData), so a
  buffered invalidation is processed on the idle tick.
- Workers-mode doc corrected: a tracked connection is acquired/released per
  batch, not held across batches.

* fix(search): read the full RESP3 map reply in FTHybridCmd (#3956)

FTHybridCmd.readReply used ReadSlice, which reads a RESP3 map header's
length as an element count and consumes only half its frames, leaving the
connection desynced. Peek the type and read a map with ReadReply, like the
other FT.* parsers.

* fix(csc): retry uncached on mid-miss disable; guard cancelled fetches

- A coalesced miss re-runs uncached when CSC serving is disabled after the miss
  was reserved (RESP3 downgrade / CLIENT TRACKING loss during a conn re-init),
  via an internal retry-uncached sentinel that processCached catches, instead of
  surfacing a spurious pool.ErrClosed for a valid cacheable read.
- A caller whose context cancels mid-fetch no longer races the coalescer: a CAS
  interlock (claimAbandon/claimApply) hands the Cmder to exactly one of the caller
  or the applying worker, so the worker never writes a Cmder the caller has taken
  back. The reply is still classified and published to the shared cache, and a
  cancelled Get returns the context error deterministically (matching the
  non-coalesced path).
- A fetch after coalescer shutdown returns pool.ErrClosed instead of hanging on a
  post-drain enqueue race.
Adds TestClassifyCachedReply, TestCSCMissReqClaimInterlock,
TestCSCMissCoalesceAbandonedFetchNoRace, TestFullDuplexDisabledMidMissRetriesUncached.

* fix(csc): drop batched deletes on flush; rebuild batcher on window change

- A full cache flush (FLUSHDB/FLUSHALL) cleared the cache but left the
  invalidation batcher's queued per-key deletes, which then fired and evicted
  entries repopulated after the flush (an extra miss within the window). Drop the
  batcher's pending queue on flush.
- A running batcher's window is fixed at creation, so a second client binding to
  the same shared handler with a stricter window kept the old cadence and its
  staleness bound did not hold. setInvalBatchWindow now drops the running batcher
  on a window change so the next invalidation starts a fresh one with the new
  window.

* ci(govulncheck): use stable Go to pick up security patches

setup-go's "1.26.x" resolved to go1.26.5, which govulncheck flags for two
standard-library vulnerabilities fixed in go1.26.6: GO-2026-6090 (crypto/tls)
and GO-2026-5972 (encoding/asn1). Track the latest stable toolchain so future
security patches are picked up automatically instead of pinning a patch.

* fix(csc): preserve queued deletes on rebuild; honor configured timeouts

- The invalidation batcher's stop path drains keys still buffered in its channel
  into the final flush, so a window-change rebuild loses no queued deletes
  (they would otherwise serve pre-invalidation values until TTL/MaxStaleness).
  Adds TestInvalBatchStopAppliesQueuedDeletes.
- ensureBatcher reads the window under the handler lock (not the caller's
  pre-lock snapshot) and returns nil at window 0, so a concurrent tighten or
  disable cannot be undone by a batcher rebuilt with the stale cadence.
- setInvalBatchWindow folds windows strictest-wins across attached clients
  (explicit 0/inline strictest, then smaller nonzero): a later, looser attach
  can no longer lengthen batching past an earlier client's staleness bound.
  The effective window resets when the last user releases.
- The miss-coalescer flush budget derives from the configured WriteTimeout +
  ReadTimeout (plus a 5s pool-Get floor) instead of a fixed 5s, so clients with
  deliberately long timeouts do not see only coalesced misses clipped early.

* fix(csc): batcher stop/release safety; FD idle release; flush budget

- enqueue/stop interlock: enqueue sends under stopMu.RLock and stop flips a
  stopped flag under stopMu.Lock before closing stopCh, so a key sent by a
  handler holding a stale batcher pointer lands before the close (applied by the
  stop-drain) and a post-stop enqueue applies inline — no delete is parked in a
  channel nothing drains. Adds TestInvalBatchEnqueueAfterStopAppliesInline.
- The batcher snapshots cache/refresh at creation and apply uses the snapshot,
  so the release-time stop-drain still lands queued deletes on a shared cache
  after releaseLocked has cleared the binding (a successor reusing that cache
  would otherwise serve pre-invalidation values until TTL/MaxStaleness).
- Full-duplex coalescer sessions release their held connection after an idle
  grace (cscFullDuplexSessionIdle, 1s) instead of holding it toward the 30s
  recycle age: at PoolSize:1 a non-cacheable command otherwise blocks until
  PoolTimeout. The next miss re-acquires; an idle coalescer holds zero
  connections. Adds TestFullDuplexSessionReleasesIdleConn.
- batchBudget includes the configured PoolTimeout alongside Write/ReadTimeout
  (5s floor for scheduling only), so a coalesced flush's pool Get waits the same
  saturation budget as the normal path, including ReadTimeout: -1 setups.

* fix(csc): close coalescer lifecycle races; honor pool budget on acquire

- cscMissCoalescer is an atomic.Pointer: processCached loads it once (a cache
  miss racing Client.Close could observe non-nil then call fetch on nil — a
  panic and a data race); stop Swaps to nil so exactly one closer wins.
- Connection acquisition in all three engines goes through acquireCtx:
  max(5s, PoolTimeout) so a configured pool budget above 5s is honored under
  temporary saturation (matching the normal command path), and cancelled when
  the coalescer stops so Close is not stalled behind a blocked pool Get. The
  workers engine's per-batch Get is split out of the batch write/read budget
  (batchBudget covers write+read only again).
- The runtime cleanup for a client dropped without Close also stops the
  coalescer workers (idempotent stopWorkers), so a forgotten client no longer
  leaks the worker goroutines and everything they retain (cache, pools).
- Document that ClientSideCacheCoalesceMisses requires the built-in LocalCache;
  with a custom Cache implementation the option is ignored.

* fix(csc): coalescer latency, shutdown and clone fixes

- fullDuplexLoop backs off only after an error end; a clean idle/recycle end
  starts the next session immediately instead of adding a flat 5ms to a miss
  already queued.
- flushBatch no longer routes the engine's bounded batch ctx through
  c.context(): with the default ContextTimeoutEnabled=false that stripped the
  deadline, and with ReadTimeout disabled a non-replying server wedged the
  worker in ReadRawReply and Close in wg.Wait.
- baseClient.clone carries the miss coalescer (atomic.Pointer Store of the
  owner's Load), so WithTimeout clones keep coalescing instead of silently
  falling back to per-caller fetches; lifecycle stays owner-only.
- Document that ClientSideCacheInvalidationBatchWindow requires the built-in
  LocalCache (matching the miss-coalescing option).

* fix(csc): make FD session I/O interruptible; refresh rebuild; stat CAS

- The full-duplex session writer/reader pass the engine's cancellable session
  ctx directly instead of through c.context(), which stripped it under the
  default ContextTimeoutEnabled=false; and since no context can interrupt a
  deadline-less socket read, the supervisor's stop path now bounds the graceful
  drain — after mc.stop it waits up to the batch budget and then closes the held
  connection. Close always unblocks: promptly on a healthy drain, within the
  budget against a server that never replies.
- setRefreshQueue drops a running batcher when the refresh binding changes
  (like setInvalBatchWindow), so a client attaching refresh-on-invalidate later
  does not leave batched deletes permanently feeding a nil refresher.
- countBatch's max-batch statistic uses a CAS loop, so concurrent workers cannot
  commit a smaller maximum over a larger one.

* fix(csc): join FD supervisor before release; drain queue in GC cleanup

- The FD session joins its supervisor goroutine (supDead) after closing
  superDone and BEFORE releasing the connection: the deferred scancel makes
  sctx.Done() ready at return, and a still-parked supervisor could pick it over
  the equally-ready superDone and close a healthy connection already returned to
  the pool — possibly under an unrelated command.
- The GC cleanup for a client dropped without Close drain-cancels the miss queue
  after stopping the workers (non-blocking), so a ctx-cancelled caller's queued
  request cannot strand an IN_PROGRESS reservation that would block another
  client sharing the injected cache until StaleTimeout.

* fix(csc): clear the push-probe deadline for negative ReadTimeout

A negative ReadTimeout means every WithReader skips SetReadDeadline, so the 1ms
deadline peekAndProcessPushNotifications installs for its probe stayed on the
socket and poisoned the next reply read (a spurious i/o timeout on a held
full-duplex connection, or on a pooled conn reused right after a drain). Clear
the deadline after the probe for negative-ReadTimeout clients; timeout>=0
callers re-arm their own on the next read. Fixed at the source, covering the
FD coalescer tick, the autopipeline paths and the CSC hit-drain.

* refactor(csc): full-duplex is the only miss-coalescer engine

Misses are caller-blocking — a real request waits on every fetch — so the
latency-first engine is the right one, always: a lone miss is written
immediately, batching stays opportunistic (packs only what is already queued),
and new misses stream out while earlier replies are in flight (~1 RTT per
miss, no batch phase-lock, no pool Get on the hot path).

Remove the alternatives and their configuration surface:
- the "workers" engine (N pooled conns, half-duplex batches): its per-batch
  round trip added a full RTT of tail latency to misses arriving mid-flight,
  and its batching advantage is preserved by the FD writer's opportunistic
  packing;
- the "pinned" benchmark prototype (no idle invalidation drain; could serve
  stale after an invalidation while idle; never publicly selectable);
- Options.ClientSideCacheCoalesceMode and ClientSideCacheCoalesceWorkers (both
  unreleased experimental fields) and their UniversalOptions mirrors:
  ClientSideCacheCoalesceMisses now selects the one engine, no tuning needed.

CSCMissCoalesceStats.Mode becomes Active. Background invalidation traffic is
unaffected: it keeps the batching-first windowed batcher and refresh queue.

* fix(csc): held-conn probe safety and lifetime bounds

- checkForData no longer resets the connection deadline: the probe is a raw
  MSG_PEEK|MSG_DONTWAIT syscall that never touches net.Conn deadlines, and on a
  held full-duplex connection the reader's idle tick ran it CONCURRENTLY with
  the writer inside WithWriter — stripping the armed write deadline and letting
  a blocked write hang past its timeout. connCheck's own reset stays (it runs
  on idle pool-owned conns).
- peekAndProcessPushNotifications falls back to a throttled timed drain
  (TakeCscPeriodicReadPending, the CSC drainer's opaque-transport discipline)
  when socket readiness cannot be inspected (Windows; dialer wrappers exposing
  neither syscall.Conn nor NetConn), so a push addressed to the held connection
  is consumed within the fallback interval instead of sitting until the next
  reply.
- The full-duplex session's hold is bounded by the connection's REMAINING
  absolute lifetime via a new pool.Conn.ExpiresAt() accessor (jitter included),
  floored at 10ms — a near-expiry pooled connection is no longer held for a
  fresh full recycle age past the expiry the pool's reaper enforces.

* fix(csc): scope idle-drain fallback to held conns; drainer deadline hygiene

- The opaque-transport timed-drain fallback moves out of the shared
  peekAndProcessPushNotifications and into the full-duplex session tick, the
  one place a connection has no alternative drain coverage. The blanket
  fallback made every push path run throttled timed drains on uninspectable
  transports (including mock connections), perturbing sequenced push handling
  — the maintnotifications downgrade test caught it in CI. Pooled push paths
  keep their drainer/health-check coverage. The timed drain itself is factored
  into timedPushDrain, shared by both callers.
- drainPushNotifications explicitly clears the socket deadline after its
  hard-deadline probe/drain for clients with a negative ReadTimeout (which
  never re-arm): the SetDeadline reset removed from checkForData — because it
  clobbered a concurrent writer's deadline on held full-duplex connections —
  had been clearing this residue incidentally on the next drainer tick. This
  is the explicit, correctly-scoped replacement.

* refactor(csc): drop the public coalescer stats API

Observability rides the client's normal telemetry — coalesced misses fire the
otel operation-duration and error callbacks like any other command path — so
CSCMissCoalesceStats (an unreleased experimental type) duplicated that surface
for no consumer. The engine's internal counters stay for the in-package tests,
which read them directly off the coalescer.

* refactor(csc): drop the public coalescer stats API

Observability rides the client's normal telemetry — coalesced misses fire the
otel operation-duration and error callbacks like any other command path — so
CSCMissCoalesceStats (an unreleased experimental type) duplicated that surface
for no consumer. The engine's internal counters stay for the in-package tests,
which read them directly off the coalescer.

Also: acquireCtx folds in the pool's full dial budget (DialerRetries x
(DialTimeout + DialerRetryTimeout) + DialTimeout) so a slow endpoint gets its
configured dial-retry sequence instead of being cancelled mid-sequence; and the
ClientSideCacheCoalesceMisses GoDoc documents pool sizing for the held
connection (PoolSize 1 + sustained misses starves non-cacheable commands).

Also compacts the review-round comments in the touched files (invariants and
contracts preserved; narration removed).

* fix(csc): bound the recycle-path drain like the stop path

Reaching the recycle age signaled the recycle and exited the supervisor while a
reader could stay blocked in a deadline-less read on a non-replying server —
with the supervisor gone nothing would ever close the conn, and a later Close
hung in wg.Wait. The recycle path now waits up to the batch budget for the
session to drain and then closes the connection, mirroring the stop path. Also
documents that a custom DialerRetryBackoff exceeding DialerRetryTimeout should
raise PoolTimeout (an opaque callback's total delay is not observable from
options).

* fix(csc): progress-based drain backstop; departial test flake

- The stop/recycle drain backstop force-closed the conn after one fixed
  batch budget, which could cut a HEALTHY drain of a deep in-flight
  pipeline (up to cscFullDuplexDepth replies, ~in-flight x RTT,
  possibly under maintenance-relaxed timeouts). The supervisor now
  samples the in-flight queue per budget interval: any consumed reply
  extends the wait; only a zero-progress interval closes the conn.
- TestReleaseConnRemovesConnectionAfterPartialPushRead skips the benign
  case where the drain probe times out before consuming any byte: the
  frame is intact and re-pooling is safe; only a Put after bytes moved
  into the reader is the desync bug the test pins.

* fix(pool): clear residual read deadline in checkForData

Removing the SetDeadline reset left a prior WithReader's read deadline
armed; once expired, rawConn.Read fails fast with i/o timeout before
the non-blocking peek runs, the CSC drainer treats the error as fatal
and removes an idle-but-healthy conn, evicting its cache coverage. The
full-duplex coalescer concentrates coverage on one held conn, so one
spurious removal wiped the whole cache (WAN: hit rate 25%, throughput
cut to a third). Clear only the READ deadline: the write deadline stays
untouched because checkForData runs concurrently with command writes on
a held full-duplex connection - which is what the old full SetDeadline
reset clobbered. Regression test pins the expired-deadline case.

* fix(csc): claim FD writes; progress-aware drain backstop

- The abandon interlock gated only the reply side, so the session
  writer could serialize an abandoned caller's cmd args - a
  use-after-return on mutable args (e.g. a []byte key), with the reply
  publishable under the original cache key. The writer now claims each
  request (PENDING->WRITING) around arg serialization, releases before
  the flush, and drops abandoned requests with their reservation
  cancelled; abandonOrWait yields through a WRITING claim, which spans
  one in-memory batch encode.
- The drain backstop measures progress as completed reads (readsDone)
  instead of len(inflight), which was blind to the reader's active
  read, and its interval honors the connection's effective
  (maintenance-relaxed) read timeout via pool.Conn.EffectiveReadTimeout
  so a legitimately deep read is not cut short.
- The partial-push-read test completes the frame and requires a whole-
  frame parse before skipping: an empty reader buffer alone does not
  prove the probe consumed nothing.

* fix(csc): snapshot miss wire form at enqueue

Replace the WRITING ownership claim with a wire snapshot taken in
fetch, while the caller still owns cmd: the session writer writes only
engine-owned bytes and never reads cmd, so no ownership window spans
arg serialization (which could implicitly flush and block on the
socket), abandoning callers return immediately, and a post-abandon arg
mutation can neither reach the wire nor publish under the original
cache key. Abandoned fetches complete again, so their reservations
settle instead of stranding IN_PROGRESS behind the miss backlog.

Also: fetch's Close-race branch drains the queue after cancelling (a
request landing after the shutdown drain is not retained via a live
WithTimeout clone), and the recycle backstop honors explicitly
disabled read deadlines - it waits for the session instead of force-
closing on a zero-progress interval, escalating only once stop fires
so Close still terminates.

* fix(csc): harden FD session recycle and probe paths

- timedPushDrain reads under a hard deadline: WithReader let an active
  maintenance-relaxed timeout replace the 1ms probe cap, parking the FD
  session reader for the relaxed duration on a no-data probe while it
  held the connection. WithReaderHardDeadline bypasses relaxation and
  clears the deadline on exit (the negative-ReadTimeout cleanup block
  is now redundant and removed).
- The supervisor also watches the recycle channel, so a
  reader-triggered recycle (handoff/close-on-put) gets the same bounded
  progress-based drain backstop as the age-triggered path - a stalled
  in-flight reply can no longer postpone a requested handoff until
  Close.
- The deadline-less recycle carve-out tests rt <= 0: Options.init
  normalizes ReadTimeout -1 (indefinite) to 0, so the indefinite mode
  was still force-closed after one budget interval.

* fix(csc): epoch inval drop; refresh ownership; close order

- Queued invalidations carry the batcher epoch they were enqueued
  under; drop() bumps the epoch instead of draining, and apply skips
  stale-epoch items (queue and in-progress batch alike, so the dropCh
  signal is gone). The flush handler bumps before cache.Flush(), so a
  per-key invalidation racing in from another tracked connection after
  the flush keeps its delete - the drain-based drop could discard it
  and leave a repopulated entry stale. Dedup is per key+epoch so a
  post-flush re-arrival is not swallowed by the duplicate check.
- stopCSCRefresher clears the shared handler's refresh binding only
  while it still points at the closing client's own queue: a sibling
  client sharing the cache/processor keeps its refresher fed.
- Teardown deactivates cscActive before stopping the coalescer, and
  fetch's stop paths settle with errCSCRetryUncached so a clone's miss
  racing the teardown window re-runs uncached instead of failing with
  ErrClosed on an open pool; the mid-apply branch returns the settle
  result itself rather than discarding a successfully applied reply.

* fix(csc): serialize inval apply with drop; FD handler adapter

- apply/drop are serialized by a batcher mutex: apply snapshotting the
  epoch once let a concurrent FLUSH land mid-batch and stale-epoch
  deletes still run post-flush, evicting repopulations - the case the
  epoch exists to prevent. drop() holding the mutex means an in-flight
  batch finishes before the flush (harmless: the flush wipes it) and
  every later apply sees the new epoch.
- Both FD-session push paths (the reader's per-reply drain and
  timedPushDrain) hand handlers the nonblocking cscHandlerClient
  adapter like the background drainer does: a custom push handler
  calling Close() signaled-and-deferred instead of deadlocking on
  mc.wg.Wait while the session reader is parked inside that handler.

* fix(csc): retry-uncached drains; refresh stack; weak close

- The shutdown drain and the GC cleanup settle queued misses with
  errCSCRetryUncached, matching fetch's stop paths: a caller woken
  during the teardown window re-runs its read on the still-open pool
  instead of surfacing ErrClosed.
- The shared handler tracks refresh bindings in a stack: clearing the
  ACTIVE binding restores the next-newest live sibling instead of
  nil, so closing the newest owner no longer severs an older client's
  still-running refresher (the identity check alone only protected the
  reverse order).
- The push-handler adapter closes through the canonical *Client via a
  weak back-pointer: Client.Close also stops the cached autopipeliners,
  which baseClient.Close bypassed, leaving flush goroutines running
  against closed pools. Weak so the wrapper stays collectible and the
  drop-without-Close cleanup still fires.

* fix(csc): write-side backstop; probe gate; clone bypass

- The drain backstop honors the write side too: the interval covers the
  connection's effective write timeout (pool.Conn.EffectiveWriteTimeout)
  and the deadline-less carve-out fires when either read or write
  deadlines are disabled - a deadline-free write blocked flushing a
  large request shows no read progress by construction and must not be
  cut by an age/handoff recycle.
- The opaque-transport speculative probe runs only with the built-in
  push processor: a custom processor may surface the empty-probe
  timeout per its contract, which would remove and redial a healthy
  session on every idle probe.
- withTimeout clones whose timeouts diverge from the owner's bypass
  miss coalescing: the shared engine reads the owner's options, so a
  coalesced miss would honor the owner's deadlines, defeating
  WithTimeout. Hits and uncached fetch caching are unchanged.
- releaseLocked clears refresh/refreshStack with the binding: a client
  dropped without Close must not leave a dead queue for a successor to
  inherit or restore.
- The drop-without-Close cleanup drain settles with errCSCRetryUncached
  like every other stop path (the round-22 hunk for this block was a
  silent patch no-op; applied for real).

* fix(csc): honor lifetime jitter in the FD recycle age

Capping the recycle age at the raw ConnMaxLifetime before consulting
ExpiresAt collapsed every positive-jitter connection back to the
unjittered lifetime, re-synchronizing session recycles across clients
started together - the herd ConnMaxLifetimeJitter exists to prevent.
Bound the age by the connection's actual absolute expiry only (jitter
included), subject to the independent 30s session cap.

* fix(csc): fail the FD session on push-processor errors

Log-and-continue past a surfaced push-processor error let the reader
proceed into ReadRawReply on a possibly desynchronized stream: a
processor that consumed part of a push before failing leaves the next
bytes mid-frame, and a fragment could be applied as a caller's reply
and published to the cache. Treat the error as session-fatal (close and
remove the connection), matching drainPushNotifications - the built-in
processor surfaces only mid-frame failures, and a custom processor's
contract cannot prove no bytes were consumed.

* feat(csc): native OTel attribution for coalesced misses

A coalesced miss returned from processCached without touching
processState, so the native recorder saw zero attempts and a nil
connection (empty server.address), and engine-failed requests bypassed
the error callback that processWithRetry emits. fetch now returns the
serving session connection (recorded by the reader before the done
settle, which is the happens-before edge) and processCached stamps it
into processState as one attempt; applyAndSettle and settleErr emit the
native error callback with parity to processWithRetry and the FD
autopipeline engine's failReqs - the retry-uncached sentinel is
excluded, since its command re-runs on the fully instrumented path.

Refs the deferred-observability thread on #3965.

* fix(csc): FD miss-coalescer review fixes

- Attribute write-side failures to the held session connection: the miss
  coalescer sets req.servedBy for the whole batch before the write, so a
  WithWriter error (or ctx cancel during the inflight handoff) settles with the
  connection attribution the reply path records, not a nil conn / zero attempts.

- Tolerate fragmented push frames on held sessions: once readiness is
  established, the drain uses the longer cscDrainHardReadCap budget (as the
  background drainer does) instead of the 1ms probe cap, which could time out
  mid-frame over TLS and fatally close a healthy session. The 1ms cap is
  reserved for speculative no-data probes (opaque transports).

- Route a WithTimeout clone's CSC push-handler Close through the owner wrapper:
  the clone's cscClientWeak now points at the canonical owner, so closeCanonical
  calls owner.Close() (full CSC + autopipeliner teardown) instead of
  baseClient.Close closing shared pools while the owner's drainer runs.

* fix(csc): harden refresh-on-invalidate push drain

Two fixes to the refresh-on-invalidate reply reader, matching the
miss-coalescer / background-drainer policy:

- Propagate push-processor errors instead of logging and continuing. A
  surfaced error means bytes may have been consumed mid-frame; reading the next
  reply on a desynchronized stream could publish a push fragment under the
  wrong cache key. Returning the error aborts the batch so withConn retires the
  connection.

- Use the nonblocking cscHandlerClient adapter for the drain. This reader is
  part of the refresher's waitgroup, so a custom push handler calling Close()
  on the raw client would deadlock shutdown (Close waits on the refresher
  goroutine parked in the handler).

* fix(maintnotifications): skip endpoint DNS detect when mode is disabled (#3969)

* Update options.go

* Update options_test.go

* Update options.go

* fix flaky test

* fix(maintnotifications): skip endpoint DNS detect when mode is disabled

* perf(proto): add zero-copy semantics to scan, remove redundant data c… (#3972)

* perf(proto): add zero-copy semantics to scan, remove redundant data conversions in reader

* chore: unpin govulncheck version

* chore: require govulncheck to use latest go version

* chore: pin govulncheck and resolve go-version to stable

---------

Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>

* fix(csc): record cancel metric on coalesced miss

A caller that cancels its context while waiting on a coalesced miss returned
early without recording an error metric: the serving session still completes
the background fetch (applyAndSettle success or settleErr), so neither records
THIS caller's cancellation, undercounting the cancellation rate whenever miss
coalescing is enabled. Report it here, attributed to the caller's ctx, matching
processWithRetry.

The conn is passed as nil deliberately: req.servedBy is written by the session
and only safe to read after the req.done receive (the happens-before edge),
which has not happened on the cancellation branch, and is nil anyway when the
cancel beats the serve. The metric recorder treats a nil conn as "no peer
attributes".

* fix(csc): retry-uncached on coalescer stop-acquire

When getConn fails because acquireCtx was cancelled by mc.stop during coalescer
teardown, the session settled the pulled miss and drained the queue with the raw
context.Canceled. processCached only retries uncached on the errCSCRetryUncached
sentinel, so in-flight coalesced reads surfaced a spurious cancellation even
though the caller's context was still valid, instead of re-running on the
still-open pool like the other stop paths.

acquireCtx derives from context.Background() and is cancelled only by mc.stop
(its sole non-deadline cancel source), so a context.Canceled here uniquely means
teardown: settle first and the queue drain with errCSCRetryUncached and report
stopped so the loop exits without backing off. Genuine acquire failures (dial
error, deadline) keep the raw error and the backoff path.

* fix(csc): detect coalescer stop by signal, not error

The full-duplex miss coalescer mapped a getConn cancellation to
errCSCRetryUncached by testing errors.Is(err, context.Canceled). But getConn runs
the dialer, credentials callbacks, and init hooks, any of which can return
context.Canceled from their OWN context while the coalescer is not stopping.
Treating the error value as proof that mc.stop fired permanently exited the sole
full-duplex worker; the current miss retried uncached, but later misses still
enqueued through the still-live coalescer with no worker to settle them, hanging
deadline-free callers and leaking IN_PROGRESS reservations.

Detect teardown by checking mc.stop directly instead. mc.stop is closed only by
stopWorkers, and close(mc.stop) is ordered before acquireCtx cancels its context
(before getConn returns), so a real teardown is always visible. On stop, settle
retry-uncached and exit; on any other acquire failure — including an unrelated
context.Canceled — fail the batch and back off so the worker stays alive. This
supersedes the errors.Is check added in the previous round.

* docs(csc): note per-session Limiter for miss coalescing

ClientSideCacheCoalesceMisses holds one connection per coalescer session and
serves many misses on it, so Options.Limiter is admitted (Allow/ReportResult)
once per session — per held connection — not per coalesced miss, unlike the
plain per-command path. This is inherent to the held-connection model; a
per-miss Allow would defeat the coalescing and re-admit a connection already
held. Document it on the option so callers that need strict per-command
admission or circuit-breaking know not to enable miss coalescing.

* chore(deps): bump rojopolis/spellcheck-github-actions (#3967)

Bumps [rojopolis/spellcheck-github-actions](https://github.com/rojopolis/spellcheck-github-actions) from 0.63.0 to 0.66.0.
- [Release notes](https://github.com/rojopolis/spellcheck-github-actions/releases)
- [Changelog](https://github.com/rojopolis/spellcheck-github-actions/blob/master/CHANGELOG.md)
- [Commits](https://github.com/rojopolis/spellcheck-github-actions/compare/0.63.0...0.66.0)

---
updated-dependencies:
- dependency-name: rojopolis/spellcheck-github-actions
  dependency-version: 0.66.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix(csc): propagate drain errors on fragmented attributes

The held-session push drain (pushDrainWithin) dispatched the built-in processor
through ProcessPendingNotifications, which swallows an error that occurs AFTER
partially consuming a frame — e.g. a RESP3 attribute whose remainder arrives
slower than the hard read cap. A swallowed mid-frame error left the connection
desynchronized, so later fragments could be misread as command replies and cached
under the wrong key, or fail unrelated misses.

Use ProcessPendingNotificationsBuffered for the built-in processor, exactly as
drainPushNotifications already does, so a post-consumption error is propagated and
the caller retires the desynced session. Custom processors keep the unbuffered
call (their contract is "invoked only when notifications are known present").

* fix(csc): limit bare-socket push drain to built-in processor

Round-8 review fix (codex r3809020181).

peekAndProcessPushNotifications fired on MaybeHasData (raw socket
readiness). Over TLS, maybeHasData unwraps the transport. It then
returns true for a post-handshake control record that has zero RESP
bytes. (conn_check.go's connCheck refuses to unwrap TLS for this reason.
maybeHasData does unwrap it.) A custom NotificationProcessor then ran
ProcessPendingNotifications under the 50ms hard cap. The empty read
timed out. The FD session reader treats that timeout as fatal. It closed
a healthy connection, and its in-flight misses failed. The built-in
buffered processor accepts the empty read.

Now the bare-socket-readiness path runs only for the built-in processor.
A custom processor drains only when the reader already holds decoded
bytes (HasBufferedData), which means any TLS is already unwrapped.
Tradeoff: a push notification that arrives as bare socket readiness on a
custom and idle session waits for the next reply read, the session
recycle, or the MaxStaleness backstop. The connection stays open. This
mirrors the built-in-only guard on the opaque-transport probe in the FD
session tick.

* fix(csc): peek frame type before a custom push drain

Round-9 review fix (cursor r3811858390).

The round-8 D fix let a custom push processor drain on HasBufferedData. But
the buffered bytes can be a coalesced reply, not a push. A custom processor
invoked then could consume the reply, cache it under the wrong key, and
desync the stream. The built-in processor peeks the frame type and consumes
only push frames, but the NotificationProcessor interface does not promise
that. pushDrainWithin now peeks the frame type for a custom processor and
returns without a read when the frame is not a push. The peek is bounded by
the closure's hard read deadline.

* fix(csc): propagate custom push-drain peek errors

Round-10 review fix (codex r3812520847).

The round-9 fix peeked the frame type before it handed a buffered frame to a
custom push processor, but it returned nil on a peek error. PeekReplyType can
partially consume a fragmented RESP3 attribute (DiscardNext) before it errors,
which desyncs the stream. Swallowing the error kept the desynced connection,
so later fragments could be read as a command reply and cached under the wrong
key. The peek error now propagates, so the session fails and drops the
connection, as the built-in buffered path does. A cleanly-peeked non-push
frame still returns nil (a real reply left for the caller's read).

* fix(csc): drain miss-reader pushes through the shared safe helper

Round-11 review fix (cursor r3813795126).

The full-duplex miss reader (readOne) drained pushes with the unbuffered
ProcessPendingNotifications for both processor kinds -- a third drain site the
earlier desync fix did not reach. The built-in processor there swallowed a
mid-frame DiscardNext error on a fragmented attribute, and a custom processor
got no frame-type peek, so either could leave the stream misaligned; readOne
then read the shifted bytes as the reply and cached them under the wrong key.
Both drain paths now share drainPushFrames: the built-in uses the Buffered
variant (mid-frame errors propagate) and a custom processor is handed only a
confirmed push frame (peek first, propagate the peek error). The helper sets no
read deadline, so readOne's following ReadRawReply is unaffected.

* fix(csc): retire conn on pre-command drain desync; refresh drain helper

Two review fixes.

Pre-command push-drain desync (round-13 audit): _process drains pending
pushes before executing a command, then reads the reply on the same conn and,
on the CSC-miss capture path, caches it. A fragmented RESP3 frame straddling
the drain's short hard cap left the reader mid-frame; the error was logged and
ignored, so the reply read consumed the residue and could cache it under the
wrong key. _process now closes the conn and retries on a fresh one instead of
ignoring the error (peekAndProcessPushNotifications already propagated
mid-frame errors via drainPushFrames).

Refresh reader drain (cursor r3813795126 / r3815669446): the refresh reply
reader still drained pushes with the unbuffered ProcessPendingNotifications,
a fourth drain site that could swallow a mid-frame desync before ReadRawReply
and publish a push fragment under the wrong cache key. It now routes through
the shared drainPushFrames helper, like the idle-tick and miss-reader paths.

* fix(csc): stop the refresh worker on GC cleanup

Round-14 review fix (codex r3817196217).

When refresh-on-invalidate is enabled and the caller drops the client without
Close, the runtime cleanup stopped the miss coalescer and the invalidation
drainer but never signaled the refresh worker. runCSCRefresher then stayed
parked on its ticker/queue, holding the baseClient, cache, and pools -- a
goroutine and memory leak that defeated the drop-without-Close safety net.
cscRegisterCleanups now captures the refresh handle and signals it, idempotent
and non-blocking (a new cscRevalidateHandle.signalStop guarded by sync.Once, so
Close and the cleanup cannot double-close). stopCSCRefresher uses the same
signal.

* feat(csc): add refresh-failure metric

Adds RefreshFailed to CSCRefreshStats -- the refresh-failure counter the HLD
lists (go-redis section 4.1). A refresh round trip that errors increments it;
those keys stay evicted and a later read repopulates them, so a rising count is
the signal that refresh-on-invalidate is degrading to plain eviction. Counted
per errored batch, not per key.

* fix(csc): drain pending pushes before reading the reply

The CSC miss and refresh readers drained push notifications with the
Buffered variant, which stops the instant the reader buffer empties. If a
second invalidation was still on the socket ahead of the command reply,
ReadRawReply read that push as the reply and cached it under the wrong
key -- a one-frame shift that then cascaded to later replies. The readers
now drain in BLOCKING mode: they block on the socket and skip push frames
until a non-push frame (the reply) is next, the same non-buffered
discipline the full-duplex reader already uses. PeekReplyType is
attribute-aware, so a fragmented RESP3 attribute needs no separate
buffered scan. A swallowed boundary-peek TIMEOUT is caught by the reader's
shared read deadline (ReadRawReply hits the same expired deadline and
fails the session); a swallowed non-timeout peek error would need a
malformed attribute mid-push (a server protocol bug), so this path is no
weaker than the full-duplex reader. Probe and idle paths keep the Buffered
variant so they never block. A socket-pair regression test pins the frame
order.

Also in this change:

- GC cleanup unbinds the dropped client's refresh queue from a shared
  invalidate handler (clearRefreshQueue), so a surviving sibling's
  refresh-on-invalidate keeps working instead of feeding a stopped queue.
  A refresh queue is only created inside attachSharedTrackingCSC, which
  also builds the drain handle, so the handle is always present when the
  queue is; a Conn() clone bails before the refresher starts.

- Record usedAt on deadline-free (negative-timeout) reads and writes, so
  a long full-duplex session under ReadTimeout/WriteTimeout=-2 is not
  misjudged as idle-expired by the pool and needlessly reconnected.

* fix(csc): harden miss-coalescing per review

- spill invalidation overflow off the full-duplex reply reader; the worker
  drains it through the same seen/pending dedup, so a burst collapses to one
  delete per key and no cache work runs on the reader
- drop the drain deadline cleanup that re-armed a relaxed timeout
  (WithReaderHardDeadline already restores the deadline)
- cap a coalesced write batch by bytes, not just count
- warn when ClientSideCacheInvalidationBatchWindow exceeds MaxStaleness

Refs #3965

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: sadd amr3e <saddamr3e@gmail.com>
Co-authored-by: Albert Huynh <huynhalb@gmail.com>
Co-authored-by: Vladislav Kotsev <vladygk8@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix(csc): address review findings on refresh, batcher, and spill

Refresh-on-invalidate:
- pass the refresher's internal 5s ctx directly to WithReader/WithWriter instead
  of c.context(ctx); c.context strips it to Background when ContextTimeoutEnabled
  is false, so with read/write timeouts disabled a stalled Redis hung the
  refresher and wedged Close (cursor/codex).
- recover() the flush batch and defer its cancel(), so a panic can't kill the
  refresher (silent permanent degradation) or leak the 5s timer.
- skip Reserve's (0,true) 'fetch uncached' targets: they wasted an MGET slot and
  registered server-side tracking for a never-cached key.

Invalidation batcher:
- hold the handler lock across drop()+Flush() and read the batcher/cache fresh
  under it, so a concurrent batcher rebuild can't leave stale-epoch deletes
  applying after the flush and evicting post-flush repopulations (cursor).
- hard-cap the overflow spill and schedule one full Flush past the cap: the
  invalidatable keyset is NOT bounded by cache size (the server tracks keys past
  local LRU eviction), so an unbounded spill could OOM (codex). Adds a test.

Other:
- FTHybridCmd: populate rawVal on the RESP2 path for RawResult parity (Copilot).
- docs: mark ClientSideCacheRefreshOnInvalidate experimental + LocalCache-only
  (Copilot); correct the HasBufferedData 'complete frame' comment; note the
  Effective*Timeout post-expiry side effect; reconcile the refresh header's
  stale-read claim; drop a stale symbol reference.

* fix(csc): follow-up review fixes (spill race, stats, refresh, acquire)

- inval batcher: set flushReq BEFORE releasing stopMu at the spill cap, so a
  concurrent stop() cannot close stopCh and let the worker exit without seeing
  the request (lost flush, stale entries).
- inval batcher: count a dedup-dropped duplicate invalidation as invalidation +
  noop, so InvalidationStats reflects incoming pushes (the duplicate ratio) with
  the window batcher on, matching the inline path.
- csc integration: unbind the refresh queue in the drainer's exit defer, so a
  SELF-DISABLE exit (custom-processor damping / RESP3 downgrade) restores a
  sibling's binding instead of silently breaking the survivor's refresh.
- miss coalescer: derive the connection-acquire budget from a custom
  DialerRetryBackoff (sum the per-attempt delays), so a backoff longer than
  DialerRetryTimeout no longer cancels Get mid-retry-sequence.

Addresses codex re-review findings on #3989.

* fix(csc): address codex re-review (relaxed-timeout race, acquire, spill stats)

- pool relaxed timeout: the counter is a relax/unrelax NOTIFICATION balance, not a
  per-I/O count. getEffective{Read,Write}Timeout no longer decrements it per call;
  a passed deadline triggers a single CAS-coordinated reset (expireRelaxedTimeout),
  so a full-duplex connection's concurrent reader/writer/drain-backstop can't drive
  it negative and wedge later relaxations, and a lost unrelax notification is
  recovered at the deadline. ClearRelaxedTimeout is clamped at zero. Adds a
  concurrent-expiry regression test.
- miss coalescer acquire budget: revert the DialerRetryBackoff summing — calling a
  randomized/stateful backoff out-of-band consumes different jitter samples than
  the pool's actual retries, so the estimate can still be wrong. Use the
  deterministic flat estimate; mc.stop + PoolTimeout bound it for custom backoffs.
- inval batcher: count invalidations superseded by the spill-cap full-Flush
  (enqueue discard + worker pending reset) so CSCRefreshStats doesn't undercount an
  invalidation flood.

Addresses the codex re-review on #3989.

* fix(pool): retire one relaxed holder on expiry

The relaxed-timeout deadline safety net full-cleared the whole window on
expiry. On a full-duplex conn the reader, writer, and drain backstop all
observe the expired deadline concurrently; a CAS on the shared deadline
already gates that to one action, but a full clear also wiped a DIFFERENT
relaxation sharing the conn -- a notification SetRelaxedTimeout outstanding
alongside an expired handoff deadline lost its window (the :566 race).

Retire exactly one holder instead: expireRelaxedTimeout decrements the
counter by one (clamped, clears the timeout values only when the last
holder leaves). To keep overlapping handoffs from leaking a permanently
relaxed conn, SetRelaxedTimeoutWithDeadline now takes a holder slot only
for the FIRST deadline-scoped window and re-arms by replacing the deadline
in place (Swap), so the eventual expiry retires the single holder and the
window clears.

Add deterministic regression tests: a notification window survives an
expired handoff deadline, and overlapping handoffs clear without a leak.

Addresses the codex conn.go:566 finding on #3989.

* fix(csc): split invalidation and deletion stats

InvalidationStats conflated two quantities and undercounted whenever the
window batcher deduped, spilled, or fell back to a full Flush, which drove
a recurring "another uncounted path" review cycle. Split them:

- Invalidations counts keys named in INCOMING pushes, tallied once at the
  handler choke point before dedup/batching.
- Deletions / DeletionsNoop count APPLIED deletes at the two cache choke
  points (DeleteByRedisKey and deleteByRedisKeyCollectingHot), so the count
  is accurate whether refresh-on-invalidate is on or off.

Invalidations - Deletions now measures dedup plus the flood full-Flush
fallback directly, so the batcher no longer needs ad-hoc per-drop counting
(removed addInvalidationStats and its three call sites). Both are exposed
on CSCRefreshStats.

Also snapshot the refresh recency horizon at ENQUEUE into
cscInvalItem.sinceToken and use it in apply, so a batch-window delay cannot
advance the live horizon and chill a key that was hot when the invalidation
arrived (it would degrade to plain eviction instead of a proactive
refresh). Value-passed, no added allocation.

Addresses the cursor csc_inval_batch.go batch-chill finding and the codex
csc_inval_batch.go stats-undercount finding on #3989.

* fix(pool): serialize relaxed-timeout mutations

SetRelaxedTimeoutWithDeadline stored its new timeout values before swapping the
deadline, so an expiry observed concurrently (a full-duplex reader/writer/drain
backstop) could CAS the old deadline to zero and clearRelaxedTimeout the fresh
values between the store and the swap -- leaving a window with a future deadline
but zeroed timeouts (active but ineffective; commands fall back to the shorter
normal timeout during a migration and can fail spuriously). Reordering alone only
moves the race.

Add relaxedMu to serialize all four mutators (Set, SetWithDeadline, Clear,
expire) so the timeout/deadline/counter atomics are always updated or cleared as
one unit. expireRelaxedTimeout re-validates the deadline under the lock (a
generation check): an expiry the caller observed lock-free is dropped if a newer
window replaced it. The read path (getEffective*/HasRelaxedTimeout) stays
lock-free and takes the lock only via expire on the rare observed expiry, so the
per-I/O hot path is unchanged. The former CAS retry loops collapse to plain
load/store under the lock.

Addresses the codex relaxed-timeout rearming-vs-expiry finding on #3989.

* fix(csc): repoint batcher to survivor before stop

The invalidation batcher captured its refresh binding at creation. When the
active refresh owner closed, clearRefreshQueue restored the surviving sibling on
h.refresh and stopped the batcher, but the stop-drain still offered evicted-hot
keys through the closing owner's queue -- whose drainer had already gone, so the
offers were dropped and the survivor never refetched them (they stayed evicted
until a reader missed). setRefreshQueue had the same staleness on a rebind.

Make cscInvalBatcher.refresh an atomic pointer and repoint it at the current
binding (survivor / new owner, nil if none) BEFORE stop() in both
set/clearRefreshQueue, so the stop-drain feeds the live refresher. An in-flight
apply already holding the old pointer feeds its current batch to the old queue --
a bounded, benign residual (offers to a stopped queue are dropped, never panic:
the queue channel is never closed).

Addresses the cursor "batcher drain feeds closed refresher" finding on #3989.

* fix(pool): relaxed timeout as one atomic snapshot

Ofek's review asked for the relaxed-timeout window to be published as one value
so a reader never sees it half-updated. Replace the mutex plus four atomics with
a single atomic.Pointer to an immutable relaxedState snapshot. Set,
SetWithDeadline, Clear, and expire install a new snapshot with a
compare-and-swap; the read path (getEffective* and HasRelaxedTimeout) does one
lock-free Load. This removes the writer-only mutex and the torn read a reader
could see between the counter bump and the timeout store.

It also fixes getEffective* returning the normal timeout on the call that
observes an expired deadline while a notification holder is still active. The
method now re-reads the snapshot after it retires the deadline holder and returns
the surviving relaxed timeout.

Addresses the codex relaxed-timeout thread and Ofek's snapshot request on #3989.

* fix(csc): bound coalesced writes, harden miss retry

Reader-miss coalescing and refresh both write a whole batch before they read the
replies, and the reader waits until the write finishes. A batch larger than the
connection write buffer (default 32 KiB) makes bufio flush in the middle of the
batch, which can deadlock the write and the read on large payloads. The batch
byte cap was 1 MiB. Bound each batch to the write buffer:

- The miss coalescer caps a batch at min(write buffer, 1 MiB). An over-budget
  request is carried in the writer's own state and sent as the first request of
  the next batch, not put back on the shared queue. So a shutdown cannot strand
  it, and it also does not widen a pre-existing reservation-strand window on the
  drop-without-Close path. The first request always goes.
- The refresh chunker applies the same byte bound next to its count cap.

Also route a coalescer session or transport failure through the normal retry
path. settleErr tags such errors as cscSessionError; processCached re-runs the
read with MaxRetries and backoff instead of a raw io.EOF to the caller. A
reply-level result (redis.Nil, WRONGTYPE, LOADING) stays untagged and is returned
as-is.

Addresses the Ofek review threads on #3989: write-before-read batches, the
refresh batch, the byte-cap overshoot, and the coalesced-miss retry path.

* fix(csc): drop ineffectual pending clear

The grabInto carry change made the pending = nil at the top of the writer loop a
dead store: grabInto now reassigns pending (with the next carry) before it is read
again. Drop the clear. No behavior change. Fixes the golangci-lint ineffassign
that CI flagged.

* fix(csc): read coalesced replies while writing the batch

The miss-coalescer wrote the whole batch and only then handed the requests to the
reader, so the reader was idle for the entire write. A batch larger than the
transport send capacity could deadlock: the server blocks writing early replies
that nobody reads while the writer blocks flushing later requests. Bounding the
batch to the write buffer did not fix this — the write buffer does not bound the
socket send buffer (a custom Dialer or a large WriteBufferSize breaks the
assumption), and with per-op timeouts disabled it can wedge.

Hand the batch to the reader BEFORE writing it (as the autopipeline full-duplex
engine already does), so the reader drains reply k while request k+1 is still
being written and relieves the server's send backpressure. The send cannot
deadlock the writer: a batch is at most cscMissBatchMax (128) and inflight is
cscFullDuplexDepth (4096) deep, the writer is sequential so every prior batch was
already flushed, and every reader-exit path either cancels the session context or
follows the writer's own exit. The byte cap remains only as a batch-size bound,
not as the deadlock fix.

Addresses the Ofek and codex/cursor write-before-read findings on #3989.

* fix(csc): stats without refresh, bound refresh write

Two refresh-path fixes from the review:

CSCRefreshStats returned an all-zero struct when the refresh queue was absent, so
with ClientSideCacheRefreshOnInvalidate off (the default) but invalidation
batching on it hid the cache-level Invalidations/Deletions/DeletionsNoop the
handler still records. Read those cache counters independently of the queue; gate
only the queue-specific counters on the queue.

The refresh writes a whole chunk before it reads the replies, so a chunk that the
server cannot drain (transport backpressure) blocks the flush. With per-op write
timeouts disabled the flush is deadline-less and the 5s batch context does NOT
interrupt it, so the refresher goroutine wedges — worse than the review reported.
Bound the write with a positive deadline (shared with the batch context via one
constant) so a stalled flush fails the refresh and the entries degrade to plain
eviction (self-healing). Full read/write concurrency in the refresher is a
possible follow-up, not done here.

Addresses the codex refresh-stats and refresh write-before-read findings on #3989.

* fix(csc): honor recycle/stop on the carry path

grabInto can defer one over-budget request as a carry, held in the writer's
pending slot and written as the first request of the next batch. While pending is
non-nil the writer took the carry branch and skipped the select that watches
recycle and mc.stop, so a producer whose commands are persistently over the byte
cap could keep the held connection busy and starve a maintenance handoff or a
lifetime recycle indefinitely (its OnPut hooks would never run).

Check recycle/stop/session-cancel (non-blocking) between batches even when a carry
is present, and settle the carry on bail — it lives only in the writer's state, so
the teardown drain, which covers inflight, would otherwise leak it and hang its
caller. Settle with the retry-uncached sentinel on recycle/stop so the caller
re-runs on the pool.

Addresses the codex carry-vs-recycle finding on #3989 (a follow-on from the carry
introduced last round).

* fix(csc): bound the refresh read too

Last round bounded the refresh WRITE so a deadline-less flush could not wedge the
refresher. The following WithReader still used the raw ReadTimeout: with per-op
timeouts disabled (options.go maps -2 to -1, which WithReader treats as no
deadline) a stalled reply or push drain parks refreshInvalidatedBatch forever, and
stopCSCRefresher waits on that goroutine, so Client.Close never returns.

Bound the read with a positive deadline (still capped by the 5s batch ctx, shared
via cscRefreshBatchTimeout), symmetric with the write, so a stall fails the refresh
and the entries degrade to plain eviction instead of hanging Close.

Addresses the cursor refresh-read-hangs-Close finding on #3989.

* fix(csc): per-chunk deadline for refresh round trips

The refresh split a window into chunks (by count and by write-buffer bytes) but
ran them all under one cscRefreshBatchTimeout context. When a window held more
than one chunk and their AGGREGATE latency exceeded the budget, later chunks
failed immediately even though each round trip was within the intended per-round-
trip timeout — four 2s chunks under a 5s budget would refresh only the first two
and leave the rest evicted.

Create the bounded context PER refreshInvalidatedBatch call so the documented
per-round-trip timeout applies to each chunk independently. The recover guard
stays at the loop level.

Addresses the codex per-chunk-deadline finding on #3989.

* fix(csc): flush snapshot cache on FLUSH invalidation

The FLUSHDB/FLUSHALL invalidation branch re-read the live h.cache under a second
RLock and flushed that, while per-key deletes use the cache pointer snapshotted at
handler entry. A last-user releaseLocked (drainer teardown / GC cleanup) can nil
h.cache between the entry RUnlock and the flush RLock, so the guarded
'if h.cache != nil' skipped the wipe entirely. On an injected shared cache
(cscOwnsCache == false) the teardown flush is gated off, so post-FLUSH entries
kept serving until TTL / MaxStaleness.

Flush the entry snapshot instead, matching the per-key branches. The second RLock
stays — it guards the batcher epoch against a concurrent rebuild — but only the
batcher is read fresh under it now; the cache uses the snapshot. The nil guard is
dropped: the entry guard already returns on a nil snapshot.

Addresses the cursor 'FLUSH can skip shared cache wipe' finding on #3989.

* fix(csc): drain all pushes before a coalesced/refresh reply

drainPushFrames in blocking mode, for a CUSTOM PushNotificationProcessor, peeked and
processed once. A second invalidation still on the socket ahead of the reply was
then read by the caller's ReadRawReply as the command value — published to the
shared cache under the wrong key and shifting every later reply. Loop peek-then-
process until a non-push (the reply) is next, matching the built-in blocking
discipline; non-blocking probes keep the single-pass behavior. Covers the coalescer
and refresh reply readers.

* fix(csc): bound in-flight coalesced wire by bytes

Miss coalescing serializes each caller's command into req.wire before it can enter
the bounded queue; arbitrarily many callers can block on the send holding a full
wire copy, and large cacheable commands (a big MGET) have no small encoded size, so
a burst could exhaust memory — the queue's item cap does not bound bytes. Track
total in-flight serialized bytes; before serializing, reserve this command's
approximate size and shed over cscMissWireBudgetBytes (8 MiB) to the pooled path
(errCSCRetryUncached), which is bounded by pool turns. Reserving before serialization
means over-budget callers never allocate the wire; a single command larger than the
budget sheds too (the pooled path runs it). TestCSCMissCoalescerWireBudget pins it.

* fix(push): consume a push whose name can't be peeked

The built-in blocking drain, on a frame already confirmed as a push, broke out of
the loop when PeekPushNotificationName failed for a non-'too-long' reason (a
non-string / malformed name). Breaking LEFT the push at the buffer head, so a
reply-expected reader (CSC refresh / miss-coalescer / the single-command pre-read
drain) then read the push as the command value — the intermittent 'redis: nil' and
cross-command reply shift under load. Fall through to ReadReply and CONSUME the
push instead (matching the name-too-long path): a genuine mid-frame read error still
surfaces at ReadReply and retires the connection, and a non-string name is
gracefully ignored (the downstream type assertion fails, so no handler runs).
Buffered probes still stop cleanly on a boundary timeout. Affects every RESP3
client's pre-read drain, not only CSC. TestProcessor_ConsumesUnpeekableNamePush
asserts the following reply is readable (the push was consumed, not left).

* fix(csc): release miss wire budget when the wire is consumed

The miss coalescer reserved wire-budget bytes before serializing a command, then
released them in a defer that fired on every fetch exit — so a caller that abandoned
after enqueue freed the budget while its wire snapshot was still retained in the
session channel / inflight deque. A burst of large abandoned misses could thus drive
the retained wire bytes far past cscMissWireBudgetBytes.

Track the WIRE's lifetime instead of the caller's: store the reservation on the
request (reserved) and release it exactly once, through a new settle helper, when
the request is consumed (applyAndSettle, settleErr, shutdown drain). fetch releases
in its defer only when the request never reached the queue (serialize error, or
Close/ctx before enqueue), guarded by an enqueued flag.

Also honor ContextTimeoutEnabled for the coalescer's enqueue and reply waits: when
it is off the ordinary command path drives socket I/O on context.Background()
(bounded by ReadTimeout, not the caller deadline), so a coalesced miss must not
surface context.DeadlineExceeded on the caller's deadline. Wait on c.context(ctx)
(caller ctx when the policy is on, Background when off); mc.stop stays the
unconditional shutdown signal and returns still report ctx.Err().

Addresses the csc wire-budget and context-timeout review findings on #3989.

* fix(csc): repopulate cache on coalescer-bail retry, order close

Two fixes on the CSC integration path.

Fallback populate: when the miss coalescer bailed (errCSCRetryUncached or a session
error) processCached retried through a nil-capture processWithRetry — it returned
the value but stored nothing, so a connection blip or a wire-budget shed left the
key uncached and every later reader missed it. Re-Reserve and fall through to the
capture path instead, so a successful retry repopulates the cache (Reserve may hand
fetching to another reader, which the capture path already handles).

Close ordering: stopBackgroundDrainer deactivated serving (cscActive=false) before
joining the refresher, but the refresher's stop-path flush re-fetches the in-window
invalidated keys and refreshInvalidatedBatch skips that work once cscActive is false
— so the last collection window was dropped, and on a SHARED (unowned) cache those
hot keys stayed evicted for siblings until a later miss. Stop the refresher FIRST
(serving still on, pool still open), then deactivate before stopping the coalescer,
which still shields a racing clone miss from an ErrClosed on the just-stopped
coalescer.

Addresses the csc fallback-populate and close-refresh-window review findings on #3989.

* fix(csc): cap per-target key bytes in the refresh queue

The refresh queue's depth cap (cscRefreshQueueDepth) bounds it by ITEM count, not
memory: with large key encodings, up to that many queued targets could pin far more
than the cache's own limit. Drop any target whose key bytes (cache key + Redis keys)
exceed cscRefreshTargetMaxBytes before enqueue — like a full-queue drop, it just
means a later reader does a normal miss fetch — bounding retained key memory to
roughly cscRefreshQueueDepth * the cap.

Addresses the csc refresh-queue-bytes review finding on #3989.

* fix(csc): drain queued refresh targets on refresher stop

The refresher's stop branch flushed only the collected window (pending); targets
still buffered in q.ch — offered but not yet collected — were abandoned. Because
stopCSCRefresher rebinds the invalidate handler away from this queue BEFORE
signalling stop, nothing else drains q.ch, so on a shared cache up to
cscRefreshQueueDepth hot entries stayed evicted for the surviving sibling until a
later reader missed them.

Extract drainQueue (the existing non-blocking window-fill, now reused by the
q.ch branch) and, on stop, loop drain+flush until the channel is empty. The
channel is quiescent at that point (no producer), so the loop is finite, and the
pool is still open (closeResources runs after the join), so the final refetch can
complete.

Addresses the codex drain-queued-refresh-targets finding on #3989.

* fix(csc): use live horizon for no-binding invalidations

An invalidation enqueued while the batcher had no refresh binding stamped
sinceToken 0. apply re-reads the refresh binding at apply time, so a refresh
client attaching in between (setRefreshQueue repoints the batcher, then
stop-drains it) applied those items against horizon 0 — treating every valid
entry as hot and offering it for refetch. That is the cold-key refresh loop the
recency horizon exists to prevent, and it re-registers cold keys with the server,
pinning its tracking table.

Stamp cscInvalNoHorizon (-1) when no binding exists at enqueue; apply falls back
to the binding's LIVE horizon (seeded at the attaching client's start), a real
recency test that can only under-refresh, never chase cold keys. LRUClock is a
monotonic sequence >= 0, so -1 cannot collide with a real horizon.
TestInvalBatcherRefreshAttachUsesLiveHorizon pins it (a cold pre-attach entry is
deleted but not offered; a hot one is).

Addresses the cursor refresh-attach-bypasses-recency-gate finding on #3989.

* fix(options): normalize nonpositive DialerRetries

Options.init normalized DialerRetries only when == 0, but the pool treats any
value <= 0 as the default (ConnPool.dialConn). A negative DialerRetries therefore
left the miss-coalescer's acquire deadline (which derives its budget from
opt.DialerRetries) computing a smaller retry count than the pool actually uses.
Normalize <= 0 so both see the same count; document the field accordingly.

Addresses the codex normalize-nonpositive-retry-counts finding on #3989.

* fix(csc): raise push-drain budget under relaxed timeouts

Every push drain ran under a hard 50ms read cap (WithReaderHardDeadline) that
deliberately bypasses maintenance relaxation. Right for a speculative probe on a
healthy conn; wrong mid-failover: relaxation is active exactly because the server
is slow, a push frame can fragment past 50ms, and a mid-frame timeout is a desync
— so the pre-command path retired a HEALTHY tracked connection (evicting its CSC
coverage) and, with MaxRetries=0, failed the command outright. The old pre-PR
drain honored relaxation via WithReader.

Add pushDrainBudget: the hard cap, raised to the connection's relaxed timeout
while relaxation is active (EffectiveReadTimeout; one-directional — a smaller
relaxed window never lowers the cap). Paid only when bytes are actually mid-frame:
an empty or completed drain returns without blocking regardless of the budget.
Pinned by TestPushDrainBudgetHonorsRelaxation.

Addresses the pre-command-drain finding in the #3989 deep review.

* fix(csc): join inval batcher on stop, wait on re-reserve loss

Two review fixes on the invalidation/miss integration paths.

Batcher join: stop() only signalled; the batcher's stop-drain and final flush ran
asynchronously after Close (or a rebuild) returned — a straggler goroutine past
Close, and on a shared injected cache a late apply could evict an entry a
successor client just repopulated. Give the batcher a done channel (closed when
run() exits) and a join(); every stop site (releaseLocked, setInvalBatchWindow,
set/clearRefreshQueue) now stops AND joins, making teardown and rebuilds
synchronous. join is safe under h.mu: run() takes no handler locks. Pinned by
TestInvalBatcherStopJoinIsSynchronous. Also resolves the join-before-tighten
review thread.

Bail-loser wait: after a coalescer session failure, waiters raced the re-Reserve
and every LOSER fell through to an independent pooled processWithRetry — one hot
key turned into N pooled requests exactly mid-recovery. A loser now waits on the
new owner (csc.Get parks on the in-progress entry, same shape as the
first-Reserve loser path) and only falls back to the pooled path after a second
Reserve loss.

Addresses the codex drain-after-close and keep-same-key-waiters findings on #3989.

* fix(csc): consume window demand nudge on refresh flush

A demand signal buffered while a window was being flushed survived the flush:
signalDemand fires only for keys in pendingSet, so the nudge referred to a key
this flush was already refetching, yet it immediately flushed the NEXT window on
its first collected key — defeating the coalescing window and inflating
DemandFlushes. flush() now consumes one buffered nudge after clearing the window
(collect runs only on the refresher goroutine, so no future-window demand can
exist yet).

Addresses the codex discard-stale-demand-signals finding on #3989.

* fix(csc): emit coalesced reply-error metric exactly once

applyAndSettle emitted the native error metric for every non-nil parsed reply
while a caller whose context expired late emitted its cancellation independently
— the same operation recorded twice with conflicting error types whenever the
cancellation raced the reply.

Centralize emission at the caller's req.done receive (emitReplyErr): reply errors
(redis.Nil, WRONGTYPE, ...) are recorded once, where the caller consumes them;
session errors and shed retries stay excluded (their processWithRetry re-run
emits its own outcome); a caller returning ctx.Err() emits only its cancellation.
req.servedBy is safe at the receive (the done receive is the field's
happens-before edge), so the metric now also carries the serving conn on the
paths that previously reported from the reader side.

Addresses the codex double reply+cancel metric finding on #3989.

* fix(csc): staleness warning covers injected LocalCache

The ClientSideCacheInvalidationBatchWindow > MaxStaleness misconfiguration
warning was gated on ClientSideCacheConfig != nil, so a pre-built cache injected
via ClientSideCache was never checked — a 10ms-staleness cache with a 500ms batch
window served pre-invalidation values 50x past its own contract, silently. When
the config is nil, read the injected *LocalCache's effective bound
(effectiveMaxStaleness) and run the same comparison.

Addresses the injected-cache warning gap in the #3989 deep review.

* fix(csc): budget refresh round trips by expected reply bytes

The refresh chunker bounded only the REQUEST side (key bytes vs the write
buffer); the reply side was unbounded. Within one chunk the server streams
replies for the early keys while the client is still writing the late ones, and
against a flow-controlling middlebox (a proxy or tunnel that stops reading when
its buffer toward the client fills — vanilla Redis instead buffers replies in
RAM) enough in-flight reply bytes stall the connection both ways: the classic
pipeline write-read deadlock, surfacing as the per-chunk deadline failing the
whole chunk and refresh silently degrading to eviction for large-value
workloads.

Budget each round trip's EXPECTED reply volume: collectHotAndDelete captures the
dying entry's value size (free, under the same shard lock) on the refetch
target, and the chunk boundary — extracted pure as cscRefreshChunkEnd and
unit-tested — now caps count, request bytes, and expected reply bytes (32 KiB,
comfortably inside typical kernel socket buffering). An estimate, not a
guarantee (a value may have grown since eviction); the per-chunk deadline stays
the backstop, and full read/write concurrency in the refresher remains the
follow-up if the deployment path is confirmed to apply per-connection
backpressure.

Refs the refresh write-before-read review threads on #3989.

* docs(csc): freshness contract, push spec, metrics TODO

Outcomes of the push-consumption review discussion, no behavior change.

- Document the invalidation-freshness contract on ClientSideCacheConfig: applied
  at arrival on full-duplex conns, at the next command on active conns, at the
  next drainer tick on idle conns; MaxStaleness is the hard bound everywhere.
- Add .claude/specs/push.md: the checkpoint consumption model and its physics
  (frame completion, TLS peek opacity, custom-processor gating), the freshness
  guarantee, and the rules that keep the model closed — no new speculative drain
  sites; features needing fresher pushes own a reader (FD-session pattern). Also
  records the planned NotificationDrainer capability follow-up.
- TODO(convergence) at the coalescer-bail re-run: start retry metrics at attempt
  1 via the explicit-start plumbing once the full-duplex branch lands here.

* fix(csc): non-blocking GC teardown, classify attributed refresh replies

The drop-without-Close GC finalizer set cscActive false BEFORE signalling the
refresher, so refreshInvalidatedBatch bailed and the stop-drain flush was a no-op
— in-window hot keys stayed evicted for a surviving sibling. And clearRefreshQueue
/ setRefreshQueue / releaseLocked called the batcher join() while holding h.mu,
blocking the GC finalizer (documented non-blocking) and stalling a sibling in
ensureBatcher on the hot path (miss-coalescer reader).

Detach the batcher under h.mu (detachBatcherLocked, stop-only) and join OUTSIDE
the lock — on the Close / self-disable / attach paths, and NOT AT ALL on the GC
finalizer, which is reordered (clearRefreshQueue -> refresher signalStop ->
deactivate) so its async stop-drain is runnable while serving is still on. The
guaranteed final-flush window stays the Close-path join; the finalizer stays
non-blocking. setInvalBatchWindow gets the same treatment.

Also: the refresher's cacheability check inspected raw[0], but a RESP3
attribute-prefixed error (|... frame) has raw[0]==RespAttr, so an error reply was
published as a false refresh success (next reader must evict + refetch).
Classify the FULL reply (cscRefreshReplyCacheable via classifyCachedReply), like
the miss-coalescer.

Addresses the GC-cleanup teardown findings and the attributed-error refresh
finding on #3989.

* fix(csc): cancel stranded miss on shutdown, charge wire, bound spill

Three bounds/lifecycle fixes on the coalescer and invalidation batcher.

Stranded miss: fetch's mc.stop wait branch cancelled the reservation and
re-drained mc.ch, but the wctx.Done (caller-deadline) branch only claimAbandoned,
leaving a post-drain request IN_PROGRESS so a later Get waited out StaleTimeout.
cancelIfStopping now cancels + re-drains on that branch too — gated on mc.stop
being observed, so a healthy late-cancel does not no-op a live reader's
fulfillCached (token-guarded cache populate).

Wire budget: cmdApproxBytes charges ~8 bytes for variable-size non-key args
(BinaryMarshaler, *string), so the 8 MiB in-flight wire budget under-counted.
reconcileWireBytes charges the delta between the estimate and the actual
len(req.wire) after serialization, keeping req.reserved in lockstep (settle
releases exactly once) so the counter reflects reality and the next reserve sheds.

Invalidation spill: the spill was bounded by item count only; a burst of distinct
large namespaced keys retained GBs below the item cap. Track retained key bytes
(cscInvalSpillMaxBytes = 8 MiB) and take the same full-Flush fallback when either
bound trips (a single oversized key trips it alone).

Addresses the shutdown-strand, wire-size-budget, and invalidation-backlog-bytes
findings on #3989.

* fix(csc): shed a coalesced miss when its real wire exceeds budget

The wire budget is reserved from cmdApproxBytes before serialization, then
reconciled to the actual size after. Round 5 only corrected the counter and kept
an over-budget wire, so N concurrent misses with undercounted variable-size args
(BinaryMarshaler, *string, net.IP) each pass the pre-serialize gate and then
charge their true size, drifting the in-flight total well past
cscMissWireBudgetBytes while every snapshot stays queued.

reconcileWireBytes now returns keep/shed: when charging the actual size would
cross the cap it rolls the delta back, leaves req.reserved at the estimate (fetch's
!enqueued defer releases exactly that), and fetch sheds to the pooled path — so the
over-budget wire is dropped, not retained in mc.ch/inflight. A shrinking delta
always keeps. Pinned by TestCSCMissCoalescerShedsWhenActualOverBudget.

Addresses the recheck-wire-budget-after-serialization finding on #3989.

* fix(csc): bound invalidation channel bytes, not just spill

Round 5 bounded the overflow spill by retained key bytes, but the fast-path
channel admits by item COUNT only: a burst of large keys — or one key larger than
the whole byte cap — retains unbounded bytes in ch before the spill path ever
engages.

Track ch retained key bytes (chBytes): enqueue adds on a successful ch send and
the worker subtracts on consume. A pre-admission gate diverts to the full-Flush
fallback when this key alone exceeds the byte cap, or when the ch bytes plus this
key would cross it, instead of admitting to ch. Pinned by
TestInvalBatcherChByteGateTripsFlush and TestInvalBatcherOversizedKeyTripsFlush.

Addresses the invalidation-byte-cap-before-channel-admission finding on #3989.

* docs(csc): note refresh-stats scope on a shared cache

CSCRefreshStats mixes cache-global counters (Invalidations/Deletions/DeletionsNoop,
read from the shared LocalCache) with this-client's-queue counters
(Enqueued/Refreshed/...). On a shared cache+processor the active refresh binding is
the last-attached client's queue, so an earlier client reports every invalidation
but ~zero refresh work — correct (it is not the refreshing client), but a naive
refresh/invalidation ratio then reads as broken. Document that the ratio is
meaningful only single-client or for the client holding the active binding. No
behavior change (scoping the counters per-client would change what Invalidations
means for existing readers).

Addresses the keep-shared-cache-refresh-statistics-in-one-scope finding on #3989.

* fix(csc): reserve invalidation channel bytes atomically

The ch byte gate was a non-atomic Load then Add-after-send: concurrent producers
(invalidations on multiple tracked conns) could each read a below-cap chBytes, all
send a large key, then all Add — overshooting cscInvalSpillMaxBytes while the
counter briefly read near zero. Reserve atomically before the send (add, and roll
back if it crosses the cap or the ch is full, mirroring reserveWireBytes); an
over-cap reservation diverts to the full-Flush fallback. Pinned by
TestInvalBatcherChByteReservationIsAtomic.

Addresses the reserve-invalidation-channel-bytes-atomically finding on #3989.

* fix(csc): generation-tag refresh demand notifications

A reader that passed signalDemand's pendingSet check just before flush cleared the
window could send its nudge AFTER the one-time drain, and that stale nudge then
flushed the NEXT (unrelated) window early — inflating DemandFlushes and defeating
the coalescing window. Stamp each nudge with the window generation (read before the
membership check); flush bumps the generation on clear; the refresher ignores a
nudge whose generation is retired (demandIsCurrent). Pinned by
TestCSCRefreshDemandGenerationIgnoresStaleSignal.

Addresses the generation-tag-demand-notifications finding on #3989.

* fix(csc): honor caller cancellation while enqueuing a miss

fetch used wctx (Background when ContextTimeoutEnabled is false) in the enqueue
select, so a caller that cancelled could not abort while merely WAITING FOR A QUEUE
SLOT — pre-I/O backpressure — even though the ordinary path passes the caller ctx to
ConnPool.Get. The enqueue-admission select now watches the original ctx; wctx stays
only on the reply wait, which maps to socket I/O and follows the timeout policy.
Pinned by TestCSCMissCoalescerEnqueueHonorsCallerCancel.

Addresses the keep-caller-cancellation-while-waiting-to-enqueue finding on #3989.

* fix(csc): don't re-run a limiter-denied coalesced miss

When the coalescer's session acquisition failed because Options.Limiter.Allow
REJECTED admission, the rejection was tagged cscSessionError like a transport
failure, so processCached re-ran the command through processWithRetry -> getConn ->
Limiter.Allow a SECOND time. A stateful or token-based limiter can permit that
second call, executing the very miss whose original admission was denied.

Split the admission from the dial (getConnLimited: getConn wraps it, byte-identical
for existing callers) so the coalescer can tell an Allow rejection apart from a
dial/transport failure. On a rejection, settle the caller with the error UNTAGGED
(settlePlain/drainQueuePlain) so processCached returns it as-is — one Allow total,
matching the ordinary path (shouldRetry is false for a generic limiter error). A
genuine transport failure keeps its cscSessionError retry behavior. Pinned by
TestGetConnLimitedSurfacesRejection.

Addresses the preserve-limiter-rejections-during-session-acquisition finding on #3989.

* fix(csc): unify teardown ordering across close and self-disable

Three interrelated CSC teardown findings, resolved by one canonical order.

New once-guarded stopCSCRefresherAndCoalescer runs: stop the coalescer (releasing
its held pool connection) -> run the refresher's final flush while cscActive is
still true -> deactivate. Called from both the clean Close path and the drainer
goroutine's self-disable defer (neither is the refresher/coalescer goroutine, so
the joins never self-block).

- Self-disable (disableCSCServing / custom-processor damping / drainer seeing
  cscActive go false) previously exited without stopping the refresher or
  coalescer — the goroutines and a held pool conn leaked for the client's life.
  It now runs the same teardown. Start guards on startCSCRefresher/
  startCSCMissCoalescer close a construction-time disable window.
- Handler-initiated close (a push handler calling Client.Close) no longer
  preemptively deactivates + stops the drainer before the canonical close; it lets
  the async close drive the refresher-first-while-active ordering (mirrors the GC
  path).
- Releasing the coalescer connection before the refresher's flush avoids a Close
  stall (up to ~160s on a small pool: refresh chunks waiting their deadline while
  the coalescer held the only conn). Reversing the old deactivate-first step is
  safe: every coalescer stop path returns errCSCRetryUncached/cscSessionError, so
  processCached re-runs on the still-open pool — no raw ErrClosed reaches callers.

Tests + red-checks in csc_test.go. Deferred: quiescing invalidation producers that
snapshotted the queue before unbind (needs a hot-path lock; the residual is a
bounded teardown-only refetch, not a stale serve) — see the review reply.

Addresses the self-disable-leak, handler-close-ordering, and
release-coalescer-before-refresh findings on #3989.

* fix(csc): preserve invalidation dedup across a size-cap flush

run()'s size-cap flush (len(pending) >= the batch cap) cleared the epoch-scoped
seen map without a full-cache Flush, so a duplicate key straddling the cap was
applied twice; with refresh-on-invalidate the first apply refetched and published
and the second evicted that fresh entry. Split flush into applyPending + clearSeen;
the size-cap path applies WITHOUT clearing seen, so per-key dedup spans the whole
window (timer/stop/full-flush still clear it). Epoch-bump was rejected: epoch is
captured at enqueue and only valid paired with a full Flush, so bumping here would
skip in-flight ch/spill items and lose their deletes. Pinned by
TestInvalBatcherSizeCapPreservesDedup.

Addresses the size-cap-flush-drops-dedup finding on #3989.

* fix(csc): bound the shutdown refresh drain on first failure

On Close with a large backlog against a dead/stalled server, the stop-drain gave
each chunk a fresh 5s deadline and continued after errors — a full queue could
delay Close ~160s (hours if large replies shrink chunks). The stop-drain now bails
after the first failed chunk (those entries are evicted with this client's coverage
on Close anyway). The normal (non-shutdown) refresh path is unchanged. Pinned by
TestCSCRefresherStopDrainBailsOnFirstFailure.

Addresses the cap-the-entire-shutdown-refresh-drain finding on #3989.

* fix(csc): bound concurrent coalesced wire serialization

cmdApproxBytes underestimates variable-size args, so many concurrent misses all
passed the approx reservation and each allocated a full wire snapshot BEFORE
reconcileWireBytes could shed — a concurrent burst transiently coexisted many
multi-MB buffers and could exhaust memory even though none stayed queued. Add a
serialize semaphore (cap = budget/batch) acquired before writeCmd and released
after the reconcile decision, always before the enqueue select, so no slot is held
across a channel send or reply wait (deadlock-free with the single worker); a
caller that can't get a slot sheds to the pooled path. Pinned by
TestCSCMissCoalescerBoundsConcurrentSerialize and the fetch-level shed test.

Addresses the bound-concurrent-serialization finding on #3989.

* fix(csc): share refresh queue into clones for demand flush

clone() (WithTimeout/WithContext) copied the miss coalescer but not
cscRefreshQueue, so a derived client's queue was nil and signalDemand no-opped —
the clone never early-flushed the owner's in-window batch, so a demand-worthy miss
waited the full cscRefreshWindow. Share the queue pointer into the clone (like the
pool hook); lifecycle stays safe — the refresher/drainer handles are owner-only
(nil in clones) so a clone only signals (nil-safe, non-blocking), never stops.
Pinned by TestCloneSharesRefreshQueueForDemand.

Addresses the clones-skip-refresh-demand-flush finding on #3989.

* fix(csc): guard invalidation deletes by fetch-issue order

Round-10 kept the batcher's per-window dedup (seen) across a size-cap flush so a
key straddling the cap could not be applied twice — the second apply would evict
the entry the first apply's refresh had just refetched. That traded a benign extra
miss for two defects: seen grew unbounded under sustained load (the timer that
clears it is deferred by every size-cap t.Reset), and a genuine re-invalidation of
a refreshed key was deduped away and served stale. Worse, keep-seen could not fix
the case where ONE cached command depends on SEVERAL Redis keys (e.g. MGET k1 k2):
a refresh triggered by k1's invalidation republishes the entry, and k2's
invalidation — a different seen entry — then re-evicts it. seen is keyed by Redis
key, so it structurally cannot reason about whether THIS entry was refetched.

Move the correctness out of dedup and into the delete. Stamp each entry with the
global cscFetchSeq value at the moment its fetch is ISSUED (Reserve), carried
through fulfill; snapshot cscFetchSeq when an invalidation is OBSERVED (enqueue).
collectHotAndDelete now keeps a Valid entry whose fetchSeq exceeds the
invalidation's snapshot: that fetch was issued after the write, so its reply
already reflects it. In-progress placeholders are still removed unconditionally.

Stamp at fetch ISSUE, not fulfill COMPLETION: invalidations and replies travel on
different connections with no ordering, so a stale reply can fulfill AFTER the
invalidate is observed (see deleteByRedisKey). A completion-order stamp would then
retain that stale value; an issue-order stamp is sound because a fetch issued after
the observe necessarily reached a server that had applied the write.

With correctness in the guard, dedup lifetime no longer matters: the size-cap path
goes back to flush() (apply + clearSeen), so seen is bounded and a genuine
re-invalidation is re-applied. A straddling stale duplicate is now a no-op via the
guard instead of via dedup.

Tests: TestCollectHotMultiKeyRefreshSurvives (the MGET case dedup can't fix),
TestCollectHotFetchOrderGuard (stale kept, genuine evicts — the case a fulfill-time
stamp fails), and TestInvalBatcherSizeCapGuardKeepsRefreshed (end-to-end, with a
DeletionStats noop delta proving the duplicate was applied and guard-skipped, not
deduped). Existing collectHotAndDelete callers pass a max snapshot to isolate their
recency/stats assertions from the guard.

Addresses the size-cap-flush-dedup regression and the multi-key re-eviction on #3989.

* fix(csc): extend fetch-order guard to in-progress fetches

A delayed invalidation applied while a replacement fetch was still in progress
removed the reservation even when entry.fetchSeq > fetchSnap — a fetch reserved
AFTER the invalidation was observed, which cannot hold the pre-write value.
Canceling it failed its racing Fulfill, woke every waiter as a miss, and could
duplicate requests / defeat miss coalescing. The guard was Valid-only; drop that
restriction so it keeps any entry (Valid or in-progress) issued after the observe.
An entry reserved at or before the observe (fetchSeq <= fetchSnap) may predate the
write and is still removed. Pinned by TestCollectHotGuardKeepsInProgress.

Addresses the preserve-newer-in-progress-fetches finding on #3989.

* fix(push): align idle CSC drain with the push-drain spec

drainPushNotifications diverged from peekAndProcessPushNotifications /
pushDrainWithin and .claude/specs/push.md on two points.

1. It drained under the raw cscDrainHardReadCap. During maintenance relaxation a
push frame can fragment past that small cap, and a mid-frame timeout is a fatal
desync that removes the connection and evicts its CSC coverage in the very
failover/migration window relaxation is meant to tolerate. Route the drain through
pushDrainBudget so the cap is raised to the relaxed timeout while relaxation is
active (unchanged otherwise).

2. It invoked a custom push processor whenever the socket was readable
(CheckForData), including kernel-only readability that over TLS can be a control
record with zero RESP bytes; the custom blocking loop then hit the hard cap and its
error — always fatal for a custom processor — retired a healthy connection. Gate the
custom processor on the buffered state captured BEFORE the probe: a byte the probe
just fetched from the socket/wrapper is not "already buffered", and handing it to a
blocking loop is the exact spurious retire push.md warns of. So a custom processor
drains only when RESP bytes were already in the reader buffer; kernel-only AND
probed bytes both defer to the next pass. Mirrors peekAndProcessPushNotifications
(buffered captured before any drain). The built-in buffered processor is unchanged —
it still proceeds on kernel-only readability (a boundary timeout is benign). Custom
skipping is bounded by the periodic fallback probe and the MaxStaleness backstop.

Pinned by TestDrainPushNotifications_RelaxedBudgetToleratesFragmentedFrame and
TestDrainPushNotifications_CustomProcessorSkipsKernelOnlyData.

Addresses the apply-relaxed-budgets-to-idle-drainer and enforce-custom-processor-gate
findings on #3989.

* fix(csc): carry the latest fetch snapshot when deduplicating invalidations

When two writes to the same key land in one batching window with a cache refetch
between them (an entry near MaxStaleness expiring and being re-fetched), the second
write's invalidation was deduped and DROPPED, leaving the retained item with the first
write's older fetchSnap. At apply the fetch-order guard then preserved the entry
refetched BETWEEN the writes (its fetchSeq exceeded the stale snapshot), serving it
stale until the next invalidation or MaxStaleness.

The dedup now records each key's index in pending and, on a same-epoch duplicate,
carries the LATEST (max) fetchSnap onto the retained item, so the guard evicts an
entry only refetched before the most recent write. seen and pending are always reset
together, so a stored index never outlives its entry. Pinned by
TestInvalBatcherDedupKeepsLatestFetchSnap. Addresses the retain-newest-invalidation-
snapshot finding (e4xHA).

* fix(csc): cancel partial refresh reservations on a CacheSizer panic

refreshInvalidatedBatch armed its reservation-cancelling defer AFTER the Reserve loop.
A user CacheSizer panics inside Reserve; the outer refresher recovery keeps the worker
alive, but the targets reserved so far were left IN_PROGRESS with no defer installed,
so every reader of those keys blocked until StaleTimeout. Arm the defer BEFORE the
loop so a mid-loop panic cancels the partial set during unwind. Pinned by
TestRefreshInvalidatedBatchCancelsReservationsOnSizerPanic. Addresses the cancel-
partial-refresh-reservations finding (e8hzZ).

* fix(push): keep the short first-byte boundary for speculative drains

pushDrainWithin applied the relaxation-aware budget (pushDrainBudget) to the ENTIRE
drain, including the first-byte wait. Under active maintenance relaxation a TLS control
record — socket-readable with zero RESP bytes — made the speculative/pre-command drain
block for the full relaxed timeout (seconds), stalling a ready command and the idle
drainer. Bound the first-byte wait to the short cap: under relaxation, probe one byte
under the short cap first (a timeout there means no frame began, benign) and grant the
relaxed budget only to finish a frame already in flight — mirroring the idle drainer's
probe-then-drain. Pinned by TestPushDrainWithinShortBoundaryUnderRelaxation. Addresses
the keep-short-boundary-timeout finding (e8hzh).

* fix(csc): pair the batcher with the flushed cache on full invalidation

On a FLUSHDB/FLUSHALL (nil-payload) invalidation, HandlePushNotification snapshotted
the cache at entry but read h.batcher FRESH under the flush lock. A last-user release +
rebind to a different cache (A->B) running under h.mu.Lock between the snapshot and the
flush left h.batcher = B's while cache = A: dropping B's batcher bumped B's epoch and
skipped B's already-queued deletes while only A was flushed, so B served stale entries
until MaxStaleness (zero by default).

Extract fullFlush and drop the batcher ONLY when it still belongs to the cache being
flushed (h.cache == cache). An unchanged binding still drops the fresh batcher — a
same-cache batcher rebuild's queued deletes must be superseded — but a rebind no longer
drops the new binding's batcher. Pinned by
TestInvalidateHandlerFullFlushPairsBatcherWithSnapshotCache.

Addresses the P1 keep-batcher-paired-with-the-snapshotted-cache finding (fCCqu).

* fix(csc): attribute a cancelled coalesced miss to its sent connection

When a caller's context expired after the coalescer session had sent the command but
before the reply was claimed, fetch reported the cancellation with a nil connection and
returned nil, so the error/operation-duration metrics dropped the peer attributes for a
command Redis actually processed. servedBy is unsafe to read on that branch (it races
the session's write; the happens-before edge is the req.done receive, which has not
happened yet).

Add sentConn (atomic.Pointer[pool.Conn]), stored by the session when the batch is
assigned to a connection; the cancel branch reads it to attribute the metric and the
returned conn, falling back to nil only for a true pre-send cancellation. Metrics-only,
no behavior change; the exact send-vs-cancel race is not deterministically unit-pinned.

Addresses the record-sent-coalesced-misses-before-cancellation finding (fCCqy).

* fix(internal): drain pooled semaphore timer unconditionally

putSemTimer gated its stale-fire drain on `!t.Stop()` and skipped it when Stop
returned true. Stop's return cannot decide whether a fire is pending: it reports
false for an already-expired timer, but a fire can settle into the channel while
Stop runs, and Stop can report true in that window. A timer holding an
undelivered fire then went back to the pool dirty, and the next Acquire's
Reset-and-select read the stale value as an instant timeout.

Drain unconditionally with a non-blocking select/default after Stop. It is safe
under both timer-channel semantics and never hangs: after Stop the timer is
disarmed, so at most one fire can be pending; the default branch runs when none
is.

Observed in CI on Go 1.24 (oldstable, GODEBUG unset) as a spurious timeout in
TestFIFOSemaphoreNoStaleTimerFire. Not reproduced locally on go1.24.13 or
go1.25.3 across tens of thousands of iterations; the fix holds by construction,
since Stop's return cannot detect a pending fire.

* fix(csc): recover coalescer callback panics; honor pre-I/O cancel; bound reply deadline

Three coalescer robustness fixes from the #3989 review.

Recover a panicking user CacheSizer (via fulfillCached) or custom Cmder reply
parse on the full-duplex reader goroutine: applyAndSettle now defers a recover
that calls settleErr, so the panic neither crashes the process nor leaves the
caller blocked on req.done (the req is already off inflight, so the teardown
drain cannot reach it). fetch now releases the serialize slot and cancels the
cache reservation on a panic in the writeCmd window (folded into the !enqueued
defer + a serializeHeld flag), instead of leaking the slot and pinning the key
IN_PROGRESS until StaleTimeout. These are guardrails for the documented
'callbacks must not panic' contract; they mirror the refresher/invalidation-
batcher recover pattern and are not separately unit-pinned.

Honor caller cancellation while a coalesced miss waits for a session but has not
yet reached a connection (req.sentConn == nil), even when ContextTimeoutEnabled
is false. The ordinary path passes ctx to ConnPool.Get and only drops to the
policy ctx for socket I/O; the reply wait now matches that, then follows the
policy once I/O has begun.

Arm the reply read under batchBudget (write + read), not bare ReadTimeout: the
writer hands a batch to the reader before flushing it (deadlock avoidance), so a
reply read can arm while its request is still being written, and a slow/large
write within WriteTimeout would spuriously time out and tear down the session.
The drain backstop's zero-progress interval is kept strictly above batchBudget so
a legitimately slow single reply is not mistaken for a stall. This changes a
user-visible reply-timeout bound; no test drives a reply slower than ReadTimeout.

Addresses the coalescer-reader-panic (fCCni-689), fetch-panic-leak (532),
post-enqueue-cancellation (492) and reply-deadline-during-write (392) findings.

* fix(csc): safe cache identity, batcher rebind pairing, per-notification fetch order

Three invalidation-handler correctness fixes from the #3989 review.

Compare caches with sameCache() instead of ==: h.cache == cache in fullFlush
panics on every FLUSHDB/FLUSHALL when the cache's dynamic type is non-comparable
(a supported case — sameCache already exists for it), crashing the push
goroutine. Pinned by TestInvalidateHandlerFullFlushNonComparableCacheNoPanic.

Pair the windowed enqueue batcher with the snapshot cache: after a last-user
release + rebind to a different cache (A->B) between the notification snapshot and
ensureBatcher, the returned batcher belonged to B and deleted B's entries for a
push meant for A. Guard with sameCache(b.cache, cache), else fall through to the
inline delete on the snapshot; fullFlush already guarded the same pairing.

Snapshot the fetch-order sequence once per notification, not per key: a live
per-key cscFetchSeq.Load() included a fetch reserved after the push was observed,
so collectHotAndDelete wrongly evicted the fresh value / cancelled its
reservation. Mirrors cscInvalItem.fetchSnap (taken at enqueue).

The rebind and fetch-order fixes are race-dependent and not separately unit-
pinned; the CSC -race suite covers them. Known follow-up: the batcher enqueue
path (csc_inval_batch.go) still snapshots cscFetchSeq per enqueue call rather than
per notification — a deliberate, larger change left for a focused commit.

Addresses the safe-cache-identity (468), enqueue-cache-rebind (409) and
per-notification-fetch-order (434) findings.

* fix(csc): boundary-probe idle drain; document built-in-processor and hook contract

Route the background idle-drainer read through a relaxation-aware boundary probe,
mirroring pushDrainWithin: under active maintenance relaxation, confirm a RESP
byte under the short hard cap before granting the relaxed budget, so a TLS control
record (socket-readable, zero RESP bytes) cannot stall the single-goroutine
drainer for the full relaxed timeout (3068).

Document the contract instead of chasing every corner case: a custom
PushNotificationProcessor is not supported with client-side caching (CSC relies on
the built-in processor consuming kernel-only data and tolerating boundary
timeouts); the client logs a warning at init in that case. Hooks and user
callbacks must call next, must not call Close or other client control methods
(deadlock), must not panic, and must not mutate client/connection state. Behavior
is undefined otherwise. Documented on Options.ClientSideCacheConfig, AddHook,
.claude/specs/push.md, and AGENTS.md.

Addresses the custom-processor-staleness (3057) and custom-processor-gate
(push.md:35) findings by contract, and the relaxed-idle-drain-probe (3068) finding
by fix.

* Revert "fix(internal): drain pooled semaphore timer unconditionally"

This reverts commit e62712061e.

* fix(csc): publish client weak ref before the drainer starts

cscClientWeak was assigned in cscRegisterCleanups, which runs AFTER attachCSC has
already launched the background drainer. The push-handler adapter's canonical close
reads cscClientWeak.Value() on the drainer goroutine, so a push arriving in that
window both raced the assignment (a data race) and could miss the close. Publish the
weak back-reference in NewClient BEFORE attachCSC, and drop the post-attach set in
cscRegisterCleanups (writing it while the drainer is live would re-introduce the
race). Weak so a dropped *Client stays collectible and its GC cleanup still fires.

From the Copilot review of #3989.

* fix(csc): bound the invalidation batcher's pending batch by bytes

The windowed invalidation batcher size-capped its PENDING batch by item count only
(cscInvalBatchMax=4096). ch and spill bound the bytes WAITING to be consumed, but the
worker then holds each key in pending until the window or the item cap flushes, so a
stream of large keys (each under the admission limit) could retain GBs across one
window while the item count stayed far below the cap. Track retained key bytes and
flush the batch early once they cross cscInvalBatchMaxBytes (8 MiB, mirroring the
spill/refresh key-byte caps) — a normal apply, not a full-cache Flush. Pinned by
TestInvalBatcherPendingByteCapFlushes.

From the Copilot review of #3989.

* fix(csc): stop refreshes from renewing a key's read recency

LocalCache.fulfill stamps lastAccessNs unconditionally, and the refresh hot-key guard
keeps entries whose lastAccessNs is past the refresh horizon. So a background refresh
republish looked like a fresh reader access: a key invalidated faster than the horizon
tick kept refreshing itself even after every reader stopped — a self-sustaining
refetch loop contrary to the cold-key guard.

collectHotAndDelete now captures the evicted entry's reader-access token on the
refresh target, and the refresh republish restores it via LocalCache.restoreAccessToken
after fulfill, so only a real reader read (get, which re-stamps) keeps a key eligible
for refresh. A reader racing the restore only UNDER-refreshes that key (the next reader
refetches), never serves stale. Refresh runs only with the built-in *LocalCache, so the
restore is a *LocalCache method reached via a type assertion; the public Cache surface
is unchanged. Pinned by TestRefreshRepublishDoesNotRenewDemand.

From the Copilot review of #3989.

* test(csc): fix disable-mid-miss flake; correct wire-budget note

TestFullDuplexDisabledMidMissRetriesUncached read cscMissCoalescer.Load().fetch after
disableCSCServing, whose drainer teardown asynchronously swaps the coalescer to nil —
a scheduler delay made the Load() nil and the test nil-deref. Capture the coalescer
BEFORE disabling and call the captured pointer (it still honors the serving-off flag).

Correct TestCSCMissCoalescerBoundsConcurrentSerialize's comment: the serialize
semaphore bounds the NUMBER of concurrent snapshots, not their bytes — peak memory is
cap x per-snapshot size, not the wire budget, because a single undercounted argument
still allocates its full snapshot before reconcileWireBytes sheds it. The per-request
byte ceiling is a deferred follow-up; the comment no longer overclaims the bound.

From the Copilot review of #3989.

* fix(csc): snapshot invalidation fetch order once per push

The windowed invalidation batcher snapshotted cscFetchSeq inside enqueue, i.e. once
PER KEY. A push that invalidates several keys therefore stamped a different fetch-order
snapshot on each cscInvalItem, so a fetch reserved after the push was observed but
before the loop reached a later key was not treated as newer for that key: the
fetch-order guard (cscInvalItem.fetchSnap, checked at apply in collectHotAndDelete)
could then evict a fresh value or cancel an in-progress reservation on a straddling
duplicate. The inline (window==0) delete path already snapshotted once per push.

The push handler now takes fetchSnap := cscFetchSeq.Load() once, before the enqueue
loop, and passes it to enqueueAt, which stamps that shared snapshot on every key in
the push. enqueue keeps the per-call snapshot for single-key/test callers. Pinned by
TestInvalBatcherEnqueueAtStampsSuppliedFetchSnap.

From the review of #3989.

* docs(csc): correct the miss-coalescer serialize-bound comment

The cscMissMaxConcurrentSerialize comment claimed the concurrent set keeps peak memory
at O(budget) with at most a few batch-sized snapshots. That overclaims: reserveWireBytes
gates on cmdApproxBytes, which undercounts a variable-size argument, so a single command
whose encoded form far exceeds the estimate (e.g. an encoding.BinaryMarshaler returning
hundreds of MB) is fully serialized before reconcileWireBytes sheds it. The semaphore
bounds the COUNT of concurrent snapshots, not their bytes; the true worst-case transient
peak is cap x (largest single serialized command). A hard per-request byte ceiling
during writeCmd is a follow-up. Comment only.

From the review of #3989.

* fix(pool): keep relaxed timeout active while a holder survives

HasRelaxedTimeout did a single snapshot Load and returned false the moment a deadline
passed, even when a non-deadline (notification) holder was still active — contradicting
the surviving-holder semantics that getEffectiveReadTimeout already implements. Until an
Effective* call happened to run expireRelaxedTimeout, HasRelaxedTimeout reported no
relaxation while the connection was in fact still relaxed.

Retire the expired deadline holder once (a single CAS, never on the hot unexpired path)
and re-read the snapshot before deciding no relaxation remains, mirroring
getEffectiveReadTimeout. Only tests call HasRelaxedTimeout; there is no production
hot-path caller. Pinned by TestHasRelaxedTimeoutExpiresDeadlineAndReadsSurvivor.

This fixes the boolean; the timeout VALUE still comes from the single shared
(readNs, writeNs) pair (last-writer-wins), whose per-holder preservation is a separate
tracked follow-up.

From the review of #3989.

* fix(csc): apply the batcher fetch-order guard without refresh

When invalidation batching was enabled for a *LocalCache but refresh-on-invalidate was
off, the batcher's apply path discarded it.fetchSnap and did an unconditional
DeleteByRedisKey. A delayed invalidation whose fetch snapshot predated a refetch during
the batch window then deleted the newer value (or cancelled its in-progress fetch),
serving a spurious miss and possibly waking waiters into duplicate fetches.

Route the *LocalCache delete through the sequence-aware deleteByRedisKeyCollectingHot
with it.fetchSnap (discarding collected refresh targets, since refresh is off), so
fetchSeq > fetchSnap is honored exactly as on the inline and refresh-on paths. A
non-*LocalCache cache keeps the plain delete (no per-entry fetch sequence). Pinned by
TestInvalBatcherNoRefreshGuardKeepsRefetched.

From the review of #3989.

* fix(csc): abort an in-progress refresh flush when Close begins

A normal (timer/demand/window-full) refresh flush already running when Close arrived kept
its stopping=false snapshot for the whole run, so it ignored the now-closed h.stop and gave
each remaining chunk a fresh cscRefreshBatchTimeout. With reply budgeting producing
one-target chunks over a 512-key window, a stalled server could hold Close for tens of
minutes. The earlier F2 fix only bounded a flush ENTERED from the stop case.

Check h.stop between chunks on the non-stopping path and bail; the stopping==true stop-drain
already bounds itself by bailing after the first failed chunk. Bailed targets stay evicted
(a reader repopulates them; coverage is revoked on Close). Pinned by
TestCSCRefresherNormalFlushAbortsOnClose.

From the review of #3989.

* fix(csc): bound coalescer Close drain, guard inline invalidation

Two findings from the re-review that reached the miss-coalescer files, plus a
documented dispute:

- Close-time drain (csc_miss_coalesce_modes.go): drainBackstop renewed the wait on
  ANY per-interval progress, so a server trickling one reply per interval kept a deep
  in-flight pipeline (up to cscFullDuplexDepth) draining and blocked Close in wg.Wait
  for hours. Arm a total stopping-drain deadline once — cscMissStopDrainIntervals x
  interval — and close+return once spent, even while readsDone advances. Stated
  tradeoff: past the budget, already-accepted in-flight misses may be cancelled rather
  than completed (Close-never-wedges outranks completing the remainder). Non-stopping
  (age-out/handoff) recycle keeps the unbounded progress rule. drainBackstop is
  extracted to drainBackstopRun so the cap is unit-testable (pure move; the only
  in-body change is c.opt -> mc.c.opt).

- Inline invalidation (csc_integration.go): with refresh-on-invalidate OFF the inline
  delete ignored the fetch-order guard, so a delayed invalidation could cancel a miss
  reserved after this push was observed and wake coalesced waiters as duplicate misses.
  Route it through deleteByRedisKeyCollectingHot (cscInvalNoHorizon, fetchSnap), the
  mirror of the batcher's !canRefresh branch.

- batchBudget 5s floor: documented that the floor also anchors the drain stall interval
  (batchBudget + 1s), so it cannot be removed to honor short read timeouts without
  force-closing healthy conns; the read-deadline/stall-interval decouple is a follow-up.

Pinned by TestCSCMissStopDrainCapsTotalOnTrickle and
TestInvalidateHandlerInlineNoRefreshKeepsRefetched (both red-checked).

From the review of #3989.

* fix(csc): arm coalescer drain cap on a recycle-then-Close race

The round-6 stopping-drain cap only armed when drainBackstopRun was entered with
stopping==true. Age-out and handoff recycle enter with stopping==false, and the
normal-timeout main progress-wait did not watch mc.stop (only the deadline-less branch
did). So if Close arrived AFTER a recycle drain had already started, the supervisor
stayed on the unbounded progress path and a server trickling one reply per interval
could hang Close in wg.Wait for hours — the exact hang the cap was meant to prevent.

The main timer select now watches mc.stop on the non-stopping path (a nil channel once
stopping, so a closed mc.stop cannot busy-spin the select): a Close arriving mid-recycle
flips the drain to the stopping path and the next loop arms the total cap.

Pinned by TestCSCMissStopDrainCapsAfterCloseRaceInRecycle (ReadTimeout/WriteTimeout > 0
so the deadline-less branch is skipped and the drain sits in the main select, then
mc.stop is closed mid-drain; red-checked).

From the review of #3989.

* fix(csc): move single-key enqueue helper to tests

cscInvalBatcher.enqueue had no production caller: the push handler uses
enqueueAt with a fetch snapshot taken once per push, and enqueue was only a
single-key convenience for tests. golangci-lint's unused analyzer skips test
files, so it read the method as dead code and failed the lint job. Move it
verbatim into csc_inval_batch_test.go (same package); no behavior change.

* feat(csc): add ClientSideCacheRefreshRecencyWindow, default refresh-all

Options.ClientSideCacheRefreshRecencyWindow bounds
ClientSideCacheRefreshOnInvalidate to keys read within a configurable window
before their invalidation arrived. Default (0) changes to refreshing every
invalidated Valid entry regardless of recency; a positive value restores a
bounded lookback, rounded up to a guaranteed-covered multiple of the existing
200ms cscRefreshRecencyTick via a small ring of per-tick LRUClock()
snapshots (cscRecencyRing).

Tradeoff, documented on the option: refreshing re-registers a key with the
server tracking table, which manufactures the next invalidation - so the
new default turns a write-heavy, rarely-read key into a self-sustaining
refresh loop driven by write traffic until capacity eviction reaps it. The
window option is how to bound that.

cscInvalNoHorizon (-1) is reused as the "refresh everything" sentinel: it
already sits below any real lastAccessNs token, so the unchanged
deleteByRedisKeyCollectingHot comparison marks every entry hot with no
change to that function. Wired through UniversalOptions.Simple() alongside
the sibling CSC knobs.

Tests: TestCollectHotAndDeleteNoHorizonRefreshesEverything,
TestCscRecencyRingOldest, TestCscRefreshWindowTicks,
TestStartCSCRefresherRecencyWindowWiring, extended
TestUniversalOptionsSimpleCopiesCSCCoalesce.

* fix(csc): skip stop-drain refetch, guard worker teardown race

Two codex findings on #3989.

P1 (csc_refresh_on_invalidate.go:655) — Close's refresher stop-drain
used to refetch every buffered invalidated key over the network,
bailing only after the first FAILED chunk (#3965 F2). Against a
healthy server nothing fails, so that bound never triggered: a full
backlog could cost thousands of round trips, one per chunk once the
reply-byte budget shrinks each chunk to a single target. Those round
trips were always wasted — stopCSCRefresherAndCoalescer's teardown
already documented (csc_integration.go:1305-1316, pre-existing) that
invalidateAllCoverage evicts every entry this connection would have
published immediately after this flush returns, on every teardown path
(Close and both self-disable exits share the one deferred cleanup that
runs stopCSCRefresherAndCoalescer before invalidateAllCoverage). This
finishes that conclusion: flush() now skips the refetch entirely when
stopping, abandoning the buffered targets (they stay evicted; a reader
repopulates them, same outcome an errored refetch already produced).
The now-dead per-chunk stopping branches, the flush return value
(never a live signal to any caller once the skip is unconditional),
and the single-pass drain's window-cap-only shortfall are all folded
in as part of the same change: the h.stop case now loops until q.ch is
fully drained rather than leaving up to ~3584 buffered targets pinned
in the channel, since c.cscRefreshQueue is never nilled on stop.
RefreshFailed is intentionally left untouched by the skip: its doc
scopes it to errored round trips, and an abandoned-by-choice target
never attempted one.

Test: TestCSCRefresherStopDrainSkipsRefetchEntirely, replacing
TestCSCRefresherStopDrainBailsOnFirstFailure. Verified against the true
pre-fix baseline (not just the changed line): reverting to the
pre-existing code makes the erroring pooler's Get run once before the
old bail-on-first-failure logic kicks in, which the test catches.

P2 (csc_integration.go:709) — attachSharedTrackingCSC starts the
drainer before startCSCRefresher/startCSCMissCoalescer publish their
queue/handle. In principle the drainer's self-disable tick (reacting
to an async conn init's disableCSCServing) could observe cscActive
false and run stopCSCRefresherAndCoalescer, consuming workersStopOnce,
before either start function publishes — leaving that worker with no
remaining path to be stopped. This interleaving is not reachable
today: cscMinDrainInterval floors the first tick at 1ms against
synchronous, microsecond-scale construction, and nothing external
holds a reference to call Close mid-construction. That is a
timing-dependent invariant, not a structural one, so replace it with
an explicit mechanism instead of only documenting it: cscDrainHandle
gains workersMu/workersTornDown. stopCSCRefresherAndCoalescer's
teardown body sets workersTornDown=true as its first action under the
lock; each start function checks the same flag under the same lock
immediately before publishing and declines to start (launching
nothing) if teardown already ran. startBackgroundDrainer needs no
matching guard: it is what allocates cscDrainHandle, so no teardown
path can observe a non-nil handle to race against until after it
returns.

Rejected fixing this by reordering attachSharedTrackingCSC to start
the workers before the drainer, per codex's literal suggestion: that
would leave the workers unstoppable by Close if anything panicked
between the reordered start calls and the drainer's construction,
since stopBackgroundDrainer no-ops on a nil cscDrainHandle. The
current order's panic-safety property is worth keeping; the lock-based
guard closes the race without touching it.

Tests: TestCSCConstructionRaceDeclinesToStartAfterTeardown (drives
workersTornDown directly, independent of cscActive; verified to fail
if either start function's guard is reverted),
TestCSCConstructionWinsRaceStartsNormally (control),
TestCSCStopCSCRefresherAndCoalescerSetsWorkersTornDown (plumbing).

* fix(csc): widen worker-publish lock, scope limiter denial to the waker

Two cursor Bugbot findings on e2a103e5.

High: "Teardown joins unstarted CSC workers" — the workersTornDown
check in startCSCRefresher/startCSCMissCoalescer only covered the
initial field write (cscRefreshHandle / cscMissCoalescer), not the
rest of publish. startCSCRefresher still had setRefreshQueue's
predecessor-batcher join after the write, and startCSCMissCoalescer
still had its wg.Add loop after the Store. A concurrent
stopCSCRefresherAndCoalescer (the drainer's own self-disable tick,
reacting to an async conn init's disableCSCServing) could read the
just-published handle/coalescer and act on it before either had
actually finished starting: stopCSCRefresher would block on <-h.done
for a runCSCRefresher goroutine not yet launched (bounded by however
long the join took, not a permanent deadlock, but pointless to allow),
and stopCSCMissCoalescer's wg.Wait() could run concurrently with an
in-progress wg.Add — the sync.WaitGroup misuse case, which can panic.

Fix: both start functions now run their entire publish step — the
field write, the queue/batcher attach (refresher) or the wg.Add/go loop
(coalescer), and the goroutine launch — inside the SAME workersMu
critical section that stopCSCRefresherAndCoalescer's teardown body
acquires as its first action. Teardown cannot read a published
handle/coalescer until publish has fully finished (goroutines running),
because it cannot even acquire the lock until then. Holding the lock
across setRefreshQueue's batcher join is safe: that path is
internal/panic-free (lookupInvalidateHandler resolves to the built-in
push processor's plain registry lookup, never user code, since CSC
requires the built-in processor) and never re-enters workersMu.

Test: TestCSCStartRefresherHoldsLockAcrossBatcherJoin installs a
controlled predecessor batcher (no run() goroutine backing it, so its
join blocks until released) and verifies workersMu stays held —
observed via TryLock, which never blocks, so the check can't deadlock
on the lock it's inspecting — for as long as startCSCRefresher is
parked in that join, even with a concurrent teardown racing it.
Verified against e2a103e5 directly: the test correctly fails there,
since that version's lock only spans the field write. Covers the
refresher path only; the coalescer's wg.Add window gets the same fix
but has no separate test — there is no equivalent controllable
blocking hook in its publish sequence.

Medium: "Limiter denial fails queued coalesced misses" — when
Limiter.Allow rejected the miss-coalescer session's getConn, every
request already queued behind the one that triggered the check
(first) was failed with that same raw, non-retryable error
(drainQueuePlain). Those queued requests never called Allow
themselves; only first did. The existing rationale (avoid a second
Allow call that could admit the very operation just denied) is
correct for first but does not extend to requests the limiter was
never asked about.

Fix: first still gets the raw, untagged error (settlePlain, unchanged).
Every other already-queued request now gets errCSCRetryUncached
(drainQueueErr) instead, so processCached re-runs each on the ordinary
per-command path and each calls Allow independently — the same
handling this engine already uses for a session torn down mid-fetch.
drainQueuePlain is now unused and removed.

Test: TestCSCMissLimiterDenialOnlyFailsWakingRequestPlainly drives
runFullDuplexSession directly with a rejecting Limiter and one queued
follow-up request, asserting first gets the raw sentinel and the
queued request gets errCSCRetryUncached, with Allow called exactly
once. Verified to fail against the prior drainQueuePlain behavior.

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: sadd amr3e <saddamr3e@gmail.com>
Co-authored-by: Albert Huynh <huynhalb@gmail.com>
Co-authored-by: Vladislav Kotsev <vladygk8@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-09-11 11:36:13 +03:00
..