Skip to content

fix(db): forward-progress guard in both deleteCompletedBatches chunk loops (#13377) - #13570

Closed
jonlwheat2-gif wants to merge 1 commit into
diegosouzapw:fix/batches-sweep-file-ownership-chunksfrom
jonlwheat2-gif:fix/batches-sweep-forward-progress-guard
Closed

jonlwheat2-gif wants to merge 1 commit into
diegosouzapw:fix/batches-sweep-file-ownership-chunksfrom
jonlwheat2-gif:fix/batches-sweep-forward-progress-guard

Conversation

@jonlwheat2-gif

Copy link
Copy Markdown
Contributor

Summary

Stacked on #13374 (fix/batches-sweep-file-ownership-chunks). Adds the forward-progress guard that issue #13377 asks for in both for (;;) loops of deleteCompletedBatches.

Base note. This PR targets fix/batches-sweep-file-ownership-chunks (the head branch of #13374) because the two chunked loops only exist there — #13377's wording ("forward-progress guard in both for (;;) loops") is written against #13374, not against release/v3.8.51.

Why

Both loops walk completed batches in chunks of INSTANCE_SWEEP_CHUNK:

SELECT id FROM batches WHERE status = 'completed' [AND api_key_id = ?]
ORDER BY rowid LIMIT ?

Each iteration sweeps its chunk (file soft-deletes + checkpoint DELETE + batch DELETE) and then re-selects the next chunk. If a DELETE ever fails to make progress — a concurrent deleter holding the same rows, or a swallowed no-op delete — the same chunk is returned forever and the request thread spins.

Change

src/lib/db/batches.ts — +23 lines, one guard per loop:

  • remember the first id of the previous chunk;
  • throw when the next chunk starts with the same id:
deleteCompletedBatches: chunk sweep made no forward progress (first id repeated)

Failure semantics stay consistent with each mode:

Mode On throw
key ({ apiKeyId }) whole sweep runs in one outer transaction → everything rolls back
instance ({ allTenants: true }) commits per chunk (SEC-D) → earlier chunks stay committed, current chunk rolls back, rethrow

Verification

Related

…loops (diegosouzapw#13377)

Issue diegosouzapw#13377 asks for a forward-progress guard in BOTH `for (;;)` loops of
`deleteCompletedBatches` — the key-scoped loop and the instance-wide loop.

Both loops walk completed batches in chunks of `INSTANCE_SWEEP_CHUNK`:

    SELECT id FROM batches WHERE status = 'completed' [AND api_key_id = ?]
    ORDER BY rowid LIMIT ?

Each iteration sweeps its chunk (file soft-deletes + checkpoint DELETE + batch
DELETE) and then re-selects the next chunk. If a DELETE ever fails to make
progress — a concurrent deleter holding the same rows, or a swallowed no-op
delete — the same chunk is returned forever and the request thread spins.

The guard remembers the first id of the previous chunk and throws when the next
chunk starts with the same id:

    deleteCompletedBatches: chunk sweep made no forward progress (first id repeated)

Failure semantics match each mode:
- Key mode wraps the whole chunk loop in ONE outer transaction, so throwing rolls
  the entire key sweep back (all-or-nothing, unchanged).
- Instance mode commits per chunk (SEC-D), so throwing leaves earlier chunks
  committed, rolls the current chunk back, and rethrows.

Stacked on diegosouzapw#13374 (`fix/batches-sweep-file-ownership-chunks`), whose two chunked
loops this guard protects.
@jonlwheat2-gif

Copy link
Copy Markdown
Contributor Author

CI note. No pull_request workflows ran for this PR: the CI triggers are scoped to release/** and the default branch, so a base of fix/batches-sweep-file-ownership-chunks doesn't match. The only check that fired is semgrep (pass).

Checks will run once this is retargeted to release/v3.8.51 (or via #13374's own CI). Note that the release/v3.8.51 checks are currently red repo-wide — every open PR against it (#13565, #13566, #13571) shows the same failures.

Local verification of this change: the diff against #13374's head is exactly the 23 added lines in src/lib/db/batches.ts, and the file parses clean.

@diegosouzapw
diegosouzapw deleted the branch diegosouzapw:fix/batches-sweep-file-ownership-chunks September 14, 2026 16:42
@jonlwheat2-gif
jonlwheat2-gif deleted the fix/batches-sweep-forward-progress-guard branch September 30, 2026 15:38
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