Skip to content

fix(plugin-state): transact writer mutations before rollout - #4101

Merged
TorichanCapgo merged 1 commit into
mainfrom
t3/plugin-state-mutation-transactions
Nov 13, 2026
Merged

TorichanCapgo merged 1 commit into
mainfrom
t3/plugin-state-mutation-transactions

Conversation

@TorichanCapgo

@TorichanCapgo TorichanCapgo commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary (AI generated)

The first writer-only production trial of #4038 exposed replayed queue-mutation results. Fifteen distinct Worker invocations reported 112 claims within about 77 seconds, while the confirmed origin had allocated only eight claim generations. A later pg_stat_statements inspection showed eleven actual claim executions. Each successful claim must allocate one generation.

Run sync claim/ACK, garbage-collection claim/ACK, and explicit repair enqueue in separate short read-write transactions. Commit each before KV/R2 I/O. Keep shared Hyperdrive caching, public plugin reads, generation fencing, retry ceilings and provider budgets unchanged.

Select a newly provisioned KV namespace and R2 pointer bucket for the API and all nine production plugin Workers. Nonproduction bindings are unchanged. These fresh resources are required because the writer was paused and source changes during that gap were not queued.

Motivation (AI generated)

These custom PostgreSQL functions mutate leases and progress even though their callers issue SELECT statements. Replaying a cached claim makes the drain process already-settled leases, inflates its progress counters, and slows backfill. The regression tests model an outside-transaction cache and verify that successive leases and GC pages actually advance.

Business Impact (AI generated)

This fixes a blocker found before enabling any KV device reads. All production read flags remain off, the writer is paused, and replicas remain available. No response-time improvement or cost saving is claimed from this patch; those remain production rollout gates.

Test Plan (AI generated)

  • Backend lint and backend TypeScript checks.
  • 113 unit tests across 12 files, including plugin-state and resource-isolation coverage.
  • Nine focused source tests cover successive mutation leases, GC cursor progress, explicit repair enqueue, and committing before provider I/O.
  • Independent review found no blocking defect and independently passed 13 source/isolation tests.
  • Production API Wrangler dry-run build.
  • Parsed configuration verifies one matching fresh KV/R2 pair across the API and all nine production plugin environments, with nonproduction unchanged.
  • CI on 0dc8fb21d6: all 46 latest checks are complete, with 35 successful and 11 skipped. The Cloudflare Workers shard 1/2 retry passed on unchanged code after the original Docker port-bind failure before tests.
  • After deployment, compare real origin claim executions/generations with actual queue progress.
  • With old drains stopped, reset namespace-specific GC cursor/lease and backfill bookkeeping, explicitly enqueue full repair, and verify coverage, archives, leases and purges before restarting reads.
  • Verify the deployed snippet KV-learning guard, usable regional cohort telemetry and real source-change propagation before the 0.1% read canary.

No migration, permissions, primary request-path reads, global query-cache settings or queue retry budgets are changed.


Autofix with [code]smith
Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.

View guided diff Turn on auto-fix

Summary by CodeRabbit

  • Bug Fixes
    • Improved the reliability of plugin state synchronization, cleanup progress, and repair requests when operations overlap or cached results are involved.
  • Documentation
    • Clarified how state-changing operations are committed and how they relate to storage work.

@coderabbitai

coderabbitai Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Production plugin-state storage bindings change in the API and plugin configurations. Plugin-state queue mutations now run in database transactions separate from KV/R2 operations, with tests and documentation covering the transaction boundaries.

Changes: Production plugin-state bindings

Layer / File(s) Summary
Update production storage bindings
cloudflare_workers/api/wrangler.jsonc, cloudflare_workers/plugin/wrangler.jsonc
The production API configuration and all nine plugin production environments now use the new PLUGIN_STATE KV namespace ID and dated PLUGIN_STATE_POINTERS R2 bucket name.

Plugin-state mutation transactions

Layer / File(s) Summary
Run queue mutations in separate transactions
supabase/functions/_backend/triggers/plugin_state_sync.ts, tests/plugin-state-source.unit.test.ts, docs/plugin-state-kv.md
A mutate helper runs selected queue RPCs in database transactions. Tests cover sync leases, garbage-collection cursor acknowledgements, repair enqueue ordering, and storage operations outside transactions. Documentation describes this execution model.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: riderx

Merge Risk: Minimal · up to 0dc8f

Plugin-state queue mutations now commit in short transactions before KV/R2 work, which should stop replayed lease results. Production bindings move to a new KV namespace and pointer bucket. No actionable merge-blocking issue was found; complete the planned backfill verification and canary checks during rollout.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title names the main change: writer mutations (sync claim/ACK, GC claim/ACK, repair enqueue) are moved into short read-write transactions before KV/R2 I/O. It also names the affected area, plugin-…
Description check Passed The description follows the template structure. It has a Summary section covering the transaction change, a Test Plan section with checked items and steps, and a checklist. It does not include a Scree…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the main change: writer mutations (sync claim/ACK, GC claim/ACK, repair enqueue) are moved into short read-write transactions before KV/R2 I/O. It also names the affected area, plugin-state, and the rollout context. The title is concise, specific, and follows conventional commit form, so a teammate scanning history would understand the primary change.

Full details: Description check

Explanation

The description follows the template structure. It has a Summary section covering the transaction change, a Test Plan section with checked items and steps, and a checklist. It does not include a Screenshots section, which the template marks as skippable for backend changes. The description also reports manual testing only partly. Some checklist items are not addressed, such as whether documentation was updated (the diff changes docs/plugin-state-kv.md, so the box should be reviewed) and whether E2E coverage is adequate. The Test Plan lists automated checks and unchecked post-deployment items, which is acceptable for a backend change. Overall the description is mostly complete.

Resolution

Optionally tick or address the documentation checklist item, since docs/plugin-state-kv.md changes, and state whether E2E coverage applies. Manual reproduction steps are not given, but the existing test plan covers the backend change.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: a2fffbfe-789b-4ab5-9199-d79220041d4a
Commits

Reviewing files that changed from the base of the PR and between b3eb01d and 0dc8fb2.

Files selected for processing: 5

  • cloudflare_workers/api/wrangler.jsonc
  • cloudflare_workers/plugin/wrangler.jsonc
  • docs/plugin-state-kv.md
  • supabase/functions/_backend/triggers/plugin_state_sync.ts
  • tests/plugin-state-source.unit.test.ts
Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

@codspeed

codspeed Bot commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 43 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing t3/plugin-state-mutation-transactions (0dc8fb2) with main (b3eb01d)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@sonarqubecloud

Copy link
Copy Markdown

@TorichanCapgo
TorichanCapgo marked this pull request as ready for review November 13, 2026 21:43
@TorichanCapgo
TorichanCapgo merged commit 73e673d into main Nov 13, 2026
113 of 118 checks passed
@TorichanCapgo
TorichanCapgo deleted the t3/plugin-state-mutation-transactions branch November 13, 2026 21:50

This branch was successfully deployed

1 active deployment
deepsec-pr — 0dc8fb21 Deployed Nov 13, 2026 by TorichanCapgo via Scan PR changes #11087
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants