US-188: require allkeys-lru on the blob-cache Redis and warn at boot by Hazzng · Pull Request #196 · Hazzng/sql-fs

Hazzng · 2026-09-18T16:57:55Z

Closes #188

Stacks on #195.

Summary by cubic

Requires the Redis backing the blob cache to run allkeys-lru or allkeys-lfu, and warns at boot when it doesn't. Redis's default noeviction turns a full instance into a permanent outage — every write is refused, blob entries carry a 24h TTL, and the load harness measured 97.6% 5xx with no recovery — while allkeys-lru makes the same memory pressure evict cold blobs instead. Closes #188.

Boot check

Runs CONFIG GET maxmemory-policy once at boot against the data Redis; it is never awaited, never fails startup, and is skipped when no Redis is configured or when the client carries no data-plane state (blob cache and path snapshot disabled).
Accepts only allkeys-lru / allkeys-lfu; anything else logs redis_eviction_policy_unsafe at critical severity with the fix command and a policy-specific reason.
Logs redis_eviction_policy_unknown with reason:"config_get_denied" when a provider forbids CONFIG GET, and config_get_failed for other failures.

Migration

Set and persist maxmemory-policy allkeys-lru (or allkeys-lfu) on the data Redis (REDIS_DATA_URL, defaulting to REDIS_URL) before deploying.
volatile-* is not sufficient: once the instance fills with non-TTL keys it behaves like noeviction, and allkeys-random can evict a live lease.
Providers that hide the setting log config_get_denied; confirm with them that the instance evicts.
CLAUDE.md and README.md document the requirement next to REDIS_URL / REDIS_DATA_URL.

The diff also carries the atomic poison-key repair and the startup race fix: version-key repair is one Lua script that never overwrites a concurrently healed key (the loser reloads and INCRs past the winner), and runStartupMigrations uses a referenced grace timer so a driver fault at boot exits 1 instead of exiting 0 before the verdict. The role-split changeset records the two-instance measurement: a paused data plane cost 0 of 12,584 requests, versus 94.8% when the control plane is paused.

^{Written for commit 22686e5. Summary will update on new commits.}

coderabbitai · 2026-09-18T16:58:02Z

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: a046c5d6-4f50-40b8-9c0e-ae579542b47b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

cubic-dev-ai

All reported issues were addressed across 6 files

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

Hazzng · 2026-09-19T01:03:26Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5b6ea6c471

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-19T01:06:58Z

+	// shares an instance with the control role (`REDIS_DATA_URL` falls back to
+	// `REDIS_URL`), whose version counters and lock leases carry a TTL too — so
+	// it can reap a live lock lease to make room for a cached blob.
+	return { verdict: policy.startsWith("allkeys-") ? "safe" : "unsafe", policy };


Do not bless allkeys policies on a shared control Redis

When REDIS_DATA_URL is omitted, both clients target the same Redis server, so an allkeys-* policy also makes correctness-bearing exec/RW locks, version counters, and destroy tombstones eligible for eviction. In particular, the explicitly accepted allkeys-random can remove a live lease before its next renewal or remove the tombstone that prevents an absent version key from being interpreted as version 0, allowing overlapping work or stale ghost state under the exact memory pressure this change addresses. Only classify this policy as safe when the data plane is physically separated from the control Redis, or use a design that protects the control keys.

Useful? React with 👍 / 👎.

Document maxmemory-policy allkeys-lru as a deployment requirement alongside REDIS_URL / REDIS_DATA_URL in CLAUDE.md and README.md, and check it at boot with CONFIG GET maxmemory-policy. A non-allkeys policy logs redis_eviction_policy_unsafe at critical severity. The check is never awaited, never fails startup, and recognises a provider that forbids CONFIG GET as its own outcome. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…y Redis M9: `policy.startsWith("allkeys-")` blessed allkeys-random, which samples uniformly and can reap a live exec-lock lease as readily as a cold blob — the very hazard the changeset used to reject volatile-* for. That argument never distinguished the accepted policies from the rejected ones. Safe is now allkeys-lru / allkeys-lfu only, on the argument that does hold (recency and frequency make a lease renewed every 20s and a hot version counter effectively immune), and volatile-* is rejected on its real defect: it evicts only TTL-bearing keys, so an instance that fills with keys that carry none degrades to noeviction. The unsafe log line now names the reason for the policy found. Changeset, CLAUDE.md and README reconciled. M8: the boot check fired unconditionally against the data client. With REDIS_BLOB_CACHE_ENABLED=false and no path snapshot, REDIS_DATA_URL's fallback makes that the CONTROL instance, so a correct control-only deployment got a critical page whose remediation would make its leases and tombstones evictable. Gated on blobCacheEnabled || pathSnapshotEnabled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…writes (#195) * US-187: recover from a poisoned Redis version key instead of wedging writes Distinguish a structurally invalid version key (WRONGTYPE, "ERR value is not an integer") from a transport failure. A structurally invalid key is repaired in place with an epoch-stamped SET so the write publishes normally; every other INCR failure keeps the #186 deferral. The F7 destroy tombstone is exempt so a repair cannot resurrect a destroyed sandbox. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * US-167: record what a physically split data-plane Redis actually buys (#211) Replaces the "not verified: a real two-Redis deployment" line with the measurement. Control plane and data plane on separate instances, pausing each in turn: a data-plane stall costs 0 of 12,584 requests, a control-plane stall costs 94.8% of 26,643. The ~54% on the single-instance harness is a property of sharing one Redis, not a ceiling on the split. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * US-180: make vitest excludes path-independent (#210) Root-anchored "comparison/**" missed the copy inside an in-repo git worktree, sweeping a duplicate src/ suite and deliberately-excluded comparison fixtures into every pnpm test:unit run. Also exclude .claude/** outright. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * US-181: share production's error handler with the test apps (#190) Four test apps hand-rolled the leaky pre-#174 onError, so they no longer matched production and could not fail on a code-leak regression. Route them through one testErrorHandler built on clientSafeErrorCode, and guard both the shape and the absence of hand-rolled copies. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * US-168: tighten bulk-write caps and make event-loop stalls visible (#201) * US-168: tighten bulk-write caps and make event-loop stalls visible Default MAX_BULK_WRITE_BYTES to MAX_FILE_WRITE_BYTES instead of 128 MiB, add the streaming body cap POST /writeFiles never had, expose the lag histogram on /readyz, and surface a lone stall via p99.9 and a new event_loop_stall line thresholded at the Redis commandTimeout. Refs #168 — the structural exec cap / worker-thread fix stays open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * US-168: put the create route's initial files under the real bulk caps M10: POST /v1/sandboxes re-derived MAX_INITIAL_FILES / MAX_INITIAL_FILE_BYTES from MAX_BULK_WRITE_FILES / MAX_BULK_WRITE_BYTES but with its own 128 MiB default, so with the knobs unset /writeFiles capped a batch at 50 MiB while create still accepted 128 MiB of identical synchronous work. It also used a bare Number(), so a non-numeric override became NaN and removed the cap, and it enforced no per-entry limit at all. All three limits now come from lib/env.ts (MAX_BULK_WRITE_FILES moved there and gained positiveIntEnv), and the route enforces the per-file cap per entry. That is what makes CLAUDE.md's "any write surface" true; README, CLAUDE.md and the changeset now say the caps cover both batch surfaces. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * US-169: survive postgres.js throwing from its own socket-write path (#194) * US-169: survive postgres.js throwing from its own socket-write path A backend reaped mid-transaction leaves the driver flushing a buffered write to a nulled socket from a bare setImmediate, which is a fatal uncaught exception that kills the replica and every other in-flight request on it. Recognise exactly that stack frame, log it, and fail the DB awaits it stranded with EDRIVERFAULT instead of letting them hang; everything else keeps Node's default crash. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * US-169: teach the driver-fault fake dialect getSandboxEpoch main's epoch fence (#161) calls getSandboxEpoch on every script-scope transaction, so a fake dialect without it throws before the test reaches the fault path it is asserting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * US-169: fail a condemned script scope closed, and race the boot migrations M3: endScriptScope never checked #scriptTxLost. After a driver fault every later fs op throws via #assertScriptTxAlive, but just-bash swallows those into a nonzero exit rather than rejecting, so the exec path still reached endScriptScope and it COMMITTED the part of the script that had landed and reported success — the opposite of the invariant the #scriptTxLost doc states. It now delegates to the existing abort path (reject endPromise -> ROLLBACK -> reload) and rethrows the fault. M4: writeFile/appendFile awaited commitBlob — a root-`sql` statement — before #withBareTx reached the liveness assert, so a write in a condemned scope still put a statement on the wire; it was also unraced, so a fault during the blob write never settled. Assert first, route it through #db, and do the same for loadAllPaths and #refreshKnownEpoch. The sticky test now asserts the commitBlob call count too, which is what its comment already claimed. M5: runMigrations was unraced behind the new crash guard, so a driver fault at boot hung forever with no listen instead of exiting 1. Extracted runStartupMigrations() and raced it. Also fixed the script-tx-lost fake, whose `void fn({})` left the abort path's callback rejection unhandled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * US-167: record what a physically split data-plane Redis actually buys (#211) Replaces the "not verified: a real two-Redis deployment" line with the measurement. Control plane and data plane on separate instances, pausing each in turn: a data-plane stall costs 0 of 12,584 requests, a control-plane stall costs 94.8% of 26,643. The ~54% on the single-instance harness is a property of sharing one Redis, not a ceiling on the split. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * US-180: make vitest excludes path-independent (#210) Root-anchored "comparison/**" missed the copy inside an in-repo git worktree, sweeping a duplicate src/ suite and deliberately-excluded comparison fixtures into every pnpm test:unit run. Also exclude .claude/** outright. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * US-181: share production's error handler with the test apps (#190) Four test apps hand-rolled the leaky pre-#174 onError, so they no longer matched production and could not fail on a code-leak regression. Route them through one testErrorHandler built on clientSafeErrorCode, and guard both the shape and the absence of hand-rolled copies. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * US-168: tighten bulk-write caps and make event-loop stalls visible (#201) * US-168: tighten bulk-write caps and make event-loop stalls visible Default MAX_BULK_WRITE_BYTES to MAX_FILE_WRITE_BYTES instead of 128 MiB, add the streaming body cap POST /writeFiles never had, expose the lag histogram on /readyz, and surface a lone stall via p99.9 and a new event_loop_stall line thresholded at the Redis commandTimeout. Refs #168 — the structural exec cap / worker-thread fix stays open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * US-168: put the create route's initial files under the real bulk caps M10: POST /v1/sandboxes re-derived MAX_INITIAL_FILES / MAX_INITIAL_FILE_BYTES from MAX_BULK_WRITE_FILES / MAX_BULK_WRITE_BYTES but with its own 128 MiB default, so with the knobs unset /writeFiles capped a batch at 50 MiB while create still accepted 128 MiB of identical synchronous work. It also used a bare Number(), so a non-numeric override became NaN and removed the cap, and it enforced no per-entry limit at all. All three limits now come from lib/env.ts (MAX_BULK_WRITE_FILES moved there and gained positiveIntEnv), and the route enforces the per-file cap per entry. That is what makes CLAUDE.md's "any write surface" true; README, CLAUDE.md and the changeset now say the caps cover both batch surfaces. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * US-169: hold the boot race on a referenced grace timer so startup fails loudly --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * US-187: make poisoned version-key repair atomic via Lua Replace the split GET-then-SET with one EVAL that only swaps a still-poisoned key, never a concurrently healed counter or tombstone. The repair loser reloads from Postgres then INCRs past the winner. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>

Keep main's US-169 refTimer follow-up and US-187 Lua poison repair; retain this branch's eviction-policy boot check and docs. Co-authored-by: Cursor <cursoragent@cursor.com>

…-eviction-policy Co-authored-by: Cursor <cursoragent@cursor.com>

cubic-dev-ai Bot reviewed Sep 18, 2026

View reviewed changes

Hazzng mentioned this pull request Sep 18, 2026

US-192: fence every mutation on the pinned epoch, not just the composites #197

Closed

Hazzng added this pull request to stack #199 September 19, 2026 00:24

chatgpt-codex-connector Bot reviewed Sep 19, 2026

View reviewed changes

Hazzng force-pushed the fix/188-redis-eviction-policy branch from 5b6ea6c to 42867ab Compare September 19, 2026 01:09

This was referenced Sep 19, 2026

fix(sql-fs): cap the file size a sandbox exec can read or produce #200

Closed

fix(sql-fs): cap the file size a sandbox exec can read or produce #202

Merged

Hazzng force-pushed the fix/188-redis-eviction-policy branch from 42867ab to 08f42fc Compare September 19, 2026 02:21

Hazzng force-pushed the fix/188-redis-eviction-policy branch from 08f42fc to ffcc9a0 Compare September 19, 2026 04:20

Hazzng force-pushed the fix/188-redis-eviction-policy branch from ffcc9a0 to 1f3c47e Compare September 19, 2026 05:30

Hazzng and others added 2 commits September 19, 2026 15:14

Hazzng force-pushed the fix/188-redis-eviction-policy branch from 1f3c47e to 473d8c7 Compare September 19, 2026 05:45

Hazzng removed this pull request from stack #199 September 19, 2026 05:56

Hazzng added this pull request to stack #212 September 19, 2026 05:57

Hazzng and others added 3 commits September 19, 2026 17:46

Merge origin/main into fix/188-redis-eviction-policy

d57d327

Keep main's US-169 refTimer follow-up and US-187 Lua poison repair; retain this branch's eviction-policy boot check and docs. Co-authored-by: Cursor <cursoragent@cursor.com>

Merge origin/fix/187-poisoned-version-key-recovery into fix/188-redis…

22686e5

…-eviction-policy Co-authored-by: Cursor <cursoragent@cursor.com>

Hazzng merged commit c2e7741 into main Sep 19, 2026
5 checks passed

An error occurred while trying to automatically change base from fix/187-poisoned-version-key-recovery to main September 19, 2026 08:23

Navigation Menu

US-188: require allkeys-lru on the blob-cache Redis and warn at boot - #196

US-188: require allkeys-lru on the blob-cache Redis and warn at boot#196
Hazzng merged 5 commits into
fix/187-poisoned-version-key-recoveryfrom
fix/188-redis-eviction-policy

Hazzng commented Sep 18, 2026 •

edited by cubic-dev-ai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 18, 2026 •

edited

Loading

Review skipped

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Hazzng commented Sep 19, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Sep 19, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Hazzng commented Sep 18, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by cubic

Uh oh!

coderabbitai Bot commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Hazzng commented Sep 19, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Sep 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Hazzng commented Sep 18, 2026 •

edited by cubic-dev-ai Bot

Loading

coderabbitai Bot commented Sep 18, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading