Repository navigation
Prove the row-capped drain actually drains, against real PostgreSQL - #2397
Merged
Merged
Conversation
…2388) #2388 shipped with the loop covered and the statement uncovered. The drain tests inject a fake executor and prove the batch arithmetic; the SQL those batches run was only ever asserted as a string. Both halves can be right in isolation and still delete nothing, because RowCappedDeleteSql puts the cutoff, the ORDER BY and the LIMIT inside a ctid IN (...) subquery and that shape has to survive a real planner. Plants 110 expired rows and 7 fresh ones, drains with a cap of 25, and asserts the total AND the batch count. The count is the load-bearing half: one statement deleting all 110 would mean the LIMIT never bound, which is the unbounded delete #2388 replaced -- the one that times out at 300s on a real backlog and rolls back to zero rows. Asserting the batch count is also what distinguishes this from the failure #2386 measured, where a purge deleting SOMETHING looked like success for a week while the table grew. Deleting a slice and draining a backlog are different outcomes and the log could not tell them apart. Refs #2388, #2386 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Reviewed. This is a well-scoped, test-only addition to Checked the substance:
No correctness, security, or performance issues found. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#2388 landed with the loop covered and the statement uncovered.
DrainBatches_*inject a fake executor and prove the batch arithmetic.RowCappedDeleteSqlwas asserted as a string. Nothing ran the two together against PostgreSQL, so the statement's shape — cutoff bound,ORDER BY, andLIMITinside actid IN (...)subquery — had never met a real planner.Plants 110 expired rows and 7 fresh, drains with cap 25, asserts both the total (110) and the batch count (5).
The batch count is the load-bearing assertion. One statement deleting all 110 would mean the
LIMITnever bound — that is the unbounded delete #2388 replaced, the one that hits 300s on a real backlog and rolls back to zero rows. And a drain that deletes something is precisely what looked like success on the dogfood store for a week whilequery_plan_dimgrew: the 08-20 purge logged0 failedhaving peeled one thin slice. Deleting a slice and draining a backlog are different outcomes, and neither the log nor the existing tests could tell them apart.Runs under
DARLING_TEST_PGin the Darling PostgreSQL job. Test-only.