Repository navigation
test: parallel_copy gains its pytest twin, and a backend that crashed no longer passes as one that survived (#1341) - #1671
Conversation
OffgridwithJD
left a comment
There was a problem hiding this comment.
@jdatcmd Approving 2cfc2f17. Measured on PG17 with ~/pg/17-p: main's shell, this head's shell, then the twin as shipped.
| run | main's shell | this shell | twin |
|---|---|---|---|
| clean (also PG18) | — | 81/81 | 81/81 |
| harness: one backend SIGSEGV'd before each of the five survival checks | 81/81, all five "still up" PASS | 5 red, got [no ([1], N backend crashes)] with N = 1..5 |
— |
msgdrop: the worker copies "unknown error" instead of edata->message |
81/81 | 2 red | SAME 2 |
dropid1: multi_insert skips the row whose int id is 1 |
10 red, all 9 reconstruct checks PASS | 19 red | SAME 19 |
dup1 (mine, over-apply): the row whose id is 1 is written twice |
— | 19 red | SAME 19 |
dropfirst (mine): the first row of every multi_insert batch is dropped |
— | 17 red | SAME 17 |
Every red-first claim in the body reproduces. survived() reads the crash count and fails closed when it can't, and each refusal check wants its guard's own phrase.
Your two questions:
- The guard change (
test_the_suite_local_helpers_are_known_and_excluded, asserting that no suite defines its owncheck_helper) is the right tightening. My #1664 touches the docstring beside it; whichever lands second restates. - The five survival rows: please leave them undated. Our rule since #1438 is that a red that shows only under a harness edit dates nothing, because the ledger dates product mutations. They arrive
never, and the PR body records the harness measurement. I'll re-approve that restate on sight.
Non-blocking: the two FIFO checks want SQLSTATE 42809 alone. Another guard that raises 42809 (wrong object type) on the same call would pass them. Adding the refusal's own phrase, as the other refusal checks do, would close it.
🤖 Generated with Claude Code
2cfc2f1 to
c06de37
Compare
OffgridwithJD
left a comment
There was a problem hiding this comment.
@jdatcmd Re-approving the restated head c06de377 (I approved 2cfc2f17). This PR's own changes to parallel_copy.sh, test_parallel_copy.py, check_ledger.tsv and unported_suites.txt are identical line for line. In test_compare_to_bash.py the added lines are identical. Only the removed lines differ, because #1664 had already reworded that docstring on main, so the restate replaces the newer text with the same new text. The earlier measurements stand.
🤖 Generated with Claude Code
…1339) Fixes #1339 test_projection_drop_column.py:113 and :119, test_projection_parallel.py:217, test_projection_privilege.py:259, test_projection_scan_io.py:260 and test_projections.py:459 and :600, each in its shell twin too. For each finding, a one-line mutation of pgcolumnar was first run on main, where both twins stayed green (or reddened the wrong name). After the fix, the same mutation reddens the target by the same name in both twins. The pytest twin stops at a test's first red, so it can show fewer names than the shell suite. - #1339 test_projection_drop_column.py:113 (weak). The stranger arms read only 42501, so deleting the unlocked ownership check in columnar_tableam.c passed (main: 8/8 shell, twin green). New arm in both: while the owner holds SHARE, a stranger is refused at once. Under the mutation it waits out lock_timeout: "a non-owner is refused before it waits for the table lock" got 55P03. - #1339 test_projection_drop_column.py:119 (mislabelled, twin mismatch). "premise: and the two refusals are the same error" came after the two constant arms, so it could never run red; the shell suite did not have it. With both ownership checks deleted, main's twin reddened "a non-owner learns nothing". The check loses the prefix, moves first, and joins the shell suite. Under that mutation both twins now redden "and the two refusals are the same error, so neither leaks" (2BP01 against 42501). - #1339 test_projection_parallel.py:217 (mislabelled). With parallel_workers = 1 on the projection path, main reddened "premise: EXPLAIN ANALYZE launched two workers" in both twins (and the rows= premise in the shell suite). Both lose the prefix; the same mutation reddens them under their new names. - #1339 test_projection_privilege.py:259 (twin mismatch). The twin asserted 42501 (and 0A000 under the policy) twice on one error. With read_projection's aclcheck_error replaced by elog, main's shell reddened "and THAT refusal is the C check" and the twin reddened "a role with EXECUTE but no SELECT is refused read_projection". The twin's first arm now asks only "refused", as the shell does. The shell's reconstruct arms now read the SQLSTATE, as the twin did: main's shell stayed green with reconstruct's aclcheck_error replaced. Under each of the three mutations (both elogs, and the RLS refusal raised as 42501), the two twins now redden identical names. - #1339 test_projection_scan_io.py:260 (weak). With the one-page floor deleted, an empty projection is priced at 0 pages and is still chosen (main green in both). New arm in both: "and its scan is not priced at zero pages" (PG17: total cost 5.00, and 0.00 under the mutation). - #1339 test_projections.py:459 (weak). With the executor never opening the projection while EXPLAIN still names it, main was green in both. New arm in both reads EXPLAIN ANALYZE: the filter sees fewer rows than the base holds (5980 against 17980 on PG17; 17980 of 17980 under the mutation). - #1339 test_projections.py:600 (inaccurate). EXPLAIN shows that the count plans pmp (Columnar Projection: pmp), so the triage's guess that it reads the base did not hold. But nothing asserted it. With projection pages multiplied by 1000000, so the count plans the base, every :600 arm stayed green on main. New arm in both: "the old-snapshot count is planned as a projection scan". The triage's candidate mutation (GetLatestSnapshot() for the projection read only) stays green: the base liveness cache still uses the query snapshot. With both on GetLatestSnapshot(), main's shell reddened "old snapshot projection scan does not see post-snapshot rows" but the twin reddened "session A responded post-commit". The twin's two timeout arms now read whether session A answered. The shell's poll no longer accepts an ERROR that echoes the token as an answer: with session A unable to see pm, main's shell reddened the counts but never "session A opened snapshot". It does now, in both twins. Ledger: projection_drop_column is seeded (11 rows; suites_not_covered 104 -> 103). 19 rows are dated by ten mutations, 2 are renamed, and 14 are new. checks_never_observed_red 3868 -> 3863 (6141 rows = 3863 never + 2278 dated). Clean on 15-19 for every touched suite and twin. Gates: the ledger gate passes for all five suites, compare_to_bash finds every bash property covered for all five pairs, and the guard set passes (476 passed). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… stale stored digest no longer passes (#1339) #1339 listed "premise: a tdigest stored in a columnar table still answers" as mislabelled: nothing compared the stored digests, so a pgColumnar storage change could redden that premise alone, or pass it. hll's stored sketch has had a byte-for-byte arm; tdigest had none. New arm, in both harnesses: "a tdigest stored in a columnar table comes back byte for byte". The daily digests are built once into a heap table and stored from there, because a tdigest depends on the order its inputs arrive in; digests rebuilt from heap could differ for reasons that are not storage. The union-within-0.1% check keeps its premise label: a storage change that altered a digest now reddens the arm first, the suite's rule for its bounds. Measured on PG17: - Mutant: a reader returning each long value (over 512 bytes) as the previous long value read (columnar_reader.c). Main's suite: 11 + 1 failed, only "an hll stored in a columnar table comes back byte for byte"; every tdigest check passed. This branch: 11 + 2 failed, the new arm "got [f] want [t]" and the hll arm; the twin fails the same two names (read from its own records). - Rejected mutants: flipping the last byte of a long value, or zeroing its second half, makes tdigest's output function take the server down, so every later arm fails as collateral. Not used for dating. - Clean: shell 13/13 on PG15 to PG18, 7 + 6 skipped on PG19 (no hll there); the twin passes on each; the new arm passes on all five. - compare_to_bash: 15 literal matches, 0 missing. Guards 474 passed. Ledger: 6141 + 1 = 6142 rows, 3863 never (unchanged), 2278 -> 2279 dated. suites_not_covered and cluster_tests unchanged. Stacked on #1674; the ledger gate passes, and guards 476 passed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… the file's first read no longer passes (#1341) test/pytest/test_parallel_copy_dedup.py ports all 17 checks of test/parallel_copy_dedup.sh by name. Two properties the shell's header promises could break with main's shell still green; both are fixed in both twins. - The changed file differed from the first in its first bytes, and the whole file fitted in the coordinator's first 256 KiB read. With the fingerprint taken from the first read alone, main's shell passed 14 of 14. The changed file now differs only in its last row, 25,000 rows put that row past the first read (a premise checks this), and a new check, "the records hold the SHA-256 of each loaded file's bytes", compares the table's records with the server's sha256() of each file. - The header says a refused load "says what it skipped rather than returning a silent 0", but no check read the NOTICE. With the NOTICE removed, main's shell passed 14 of 14. A new check reads it. - "premise: without dedup a repeated load doubles the table, ..." and "premise: and each of those loads reported its rows" lose their prefix under #1339: dedup is pgcolumnar's own default, and forcing it on reddens both. Measured, PG17, shell and twin: - fingerprint of the first read alone: 4 red; main's shell 0. - the skip's NOTICE never raised: 1 red; main's shell 0. - dedup forced on: 4 red. The refused load committed anyway: 2 red. A fingerprint matched in any table: 3 red. The refused load reporting the rows it discarded: 1 red. Each red set is identical in both twins. - the fixture shrunk to 20,000 rows: 1 red, the size premise. - clean: 17 of 17 on PG15 to PG19 in both twins. Ledger: 17 rows, 13 dated by the 6 mutations and 4 never. suites_not_covered 74 -> 73, cluster_tests 3222 -> 3239, TESTS.md section 296. Census 4132 -> 4136 never, 2818 -> 2831 dated, 6967 rows. The ledger gate and the guards pass over eb4aa69. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… no longer passes as one that survived (#1341) test/pytest/test_parallel_copy.py ports all 81 checks of test/parallel_copy.sh by name. Ten shell checks could pass with the property broken; all are fixed in both twins. - Five survival checks read "SELECT 1", which answers again after crash recovery. With one backend killed by SIGSEGV before each, main's shell passed 81 of 81. They now also need the server log's count of killed backends unmoved since the suite began (crashes() and survived(), from avro_manifest.sh). - "atomic: a loader failure fails the whole load" and "single table: a bad row fails the whole load" took any message with "failed" in it, and every worker failure reads "worker N failed: ...". With the loader's message dropped, main's shell passed 81 of 81. They now need the loader's own cause after "failed: ". - The nine reconstruct checks compared the per-range loads with a single columnar COPY of the same file, which shares every defect of the COPY path. With multi_insert dropping the row whose id is 1, all nine passed on main. They now compare with the generator that wrote the file. - Check names built from loop variables and helper labels are literal at the call site, so the three suite-local check_ helpers are gone. test_compare_to_bash.py asserted those three existed; that arm now asserts no suite defines its own check_ helper, and it reddens on main's parallel_copy.sh. Measured, PG17, shell and twin, each red set identical in both: - a backend killed before each survival check: 5 red; main's shell 0. - the loader's message dropped: 2 red; main's shell 0. - multi_insert dropping id 1: 19 red, 9 of them reconstruct checks; main's shell 10. - a split offset on the newline: 23. The split degraded to one range: 5. The S_ISREG refusal removed: 2. The guard on max_prepared_transactions removed, bulk_parallel_writer booting on, the FIFO opener blocking, workers < 1 accepted, the key-type restriction removed: 1 each. - clean: 81 of 81 on PG15 to PG19 in both twins. Ledger: 81 rows, 49 dated by the 11 mutations and 32 never. suites_not_covered 73 -> 72, cluster_tests 3239 -> 3320, TESTS.md section 297. Census 4136 -> 4168 never, 2831 -> 2880 dated, 7048 rows. The ledger gate and the guards pass over 7e8c10f. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Restated again, onto #1668's new head (main 9ab6a4c underneath), because the registries conflicted with main. The registries are regenerated over that head by the same tooling as before; the suite and twin are unchanged; the ledger gate and the guards pass (see the commit message). Re-approval needed. Head 9ae8951. |
c06de37 to
9ae8951
Compare
OffgridwithJD
left a comment
There was a problem hiding this comment.
@jdatcmd Re-approved at 9ae8951f, on #1668's 7e8c10f7. The diff outside the bookkeeping hashes the same as the approved c06de377's. The tsv is sorted, and its census (7048 / 4168 / 2880) matches the budget.
🤖 Generated with Claude Code
@OffgridwithJD, this ports
test/parallel_copy.shtotest/pytest/test_parallel_copy.py(#1341). All 81 checks are ported by the shell's names. Ten shell checks could pass with their property broken; this fixes them in both twins.What was fixed
SELECT 1. A fresh connection answers again after crash recovery. With one backend killed by SIGSEGV before each check, main's shell passed 81 of 81. The checks now also need the server log's count of killed backends to be unchanged since the suite began (crashes()andsurvived(), as inavro_manifest.sh).worker N failed: .... With the loader's message dropped, main's shell passed 81 of 81. The checks now need the loader's own cause afterfailed:.multi_insertdropping the row whose id is 1, all nine passed on main. They now compare with the generator that wrote the file.check_helpers are gone.test_compare_to_bash.pyasserted that those three exist. That assertion is now out of date, so it now asserts that no suite defines its owncheck_helper. It fails against main'sparallel_copy.sh.Red checks under each edit and mutant
All runs are on PG17. "main" is origin/main's shell, "this shell" is the fixed shell, and "twin" is the new pytest file. In every row, the fixed shell and the twin went red on the same check names.
columnar_parallel_copy.c: the loader's message is droppedcolumnar_tableam.c:multi_insertdrops the row whose id is 1S_ISREGrefusal is removedmax_prepared_transactionsguard is removedbulk_parallel_writerdefaults to onO_NONBLOCK(#686)workers < 1is accepted"n/a" means main's shell was not run under that mutant. The edits for those mutants do not touch any check that this branch changed.
Gates
suites_not_coveredgoes from 105 to 104. The ledger gate passes on each of the five clean logs.test_harness_deps.py,test_capability_decline.pyandtest_parallel_copy.pypass with--pg-config.[].compare_to_bash.pyreports 81 literal matches: "every bash property is covered".harness_selftest.shpasses 1248 of 1248, andshellcheck -S erroris clean.cluster_testsgoes from 2677 to 2758, and TESTS.md gains section 265.Two calls for you as reviewer:
test_compare_to_bash.py::test_the_suite_local_helpers_are_known_and_excludedasserted thatparallel_copy.sh's threecheck_helpers exist. They are unrolled here, so it now asserts that no suite defines its owncheck_helper; it goes red against main'sparallel_copy.sh. Your test: nineteen #1339 entries in the harness self-tests, each measured red-first (#1339) #1664 edits the docstring beside it, so whichever lands second restates.Re-checked by me before pushing (PG17, main 5a94757): the parallel_copy S_ISREG refusal removed: main's shell, this shell and the twin each fail the same 3 names (parity; main catches this one too). For the drop-row mutant I read the saved logs: main 0 of the 9 reconstruct checks red, this shell 9.
🤖 Generated with Claude Code