Repository navigation
test: an alpha4 binary refuses what this build wrote; min/max(ts) read no data - #1301
Conversation
…d no data Two promises made today had no test of their own. commandprompt#1137 (format_downgrade.sh, test_format_downgrade.py). The stamp's claim is about two binaries, and native_format.sh stands in for alpha4 with a catalog UPDATE. These build v1.0-alpha4, write with it, upgrade to this tree, read one table, append to another, create a third, then put the alpha4 library back under the upgraded catalog: alpha4 reads the table this build only read count, sum, index fetch alpha4 refuses the appended and the new table 0A000, scan and fetch stamp set back to 1 stamps "1 1 1", 5 arms red; alpha4 answers "ok" on an index fetch of a chunk it cannot decode The shell suite is opt-in under PGC_RUN_UPGRADE=1 and invoked there, not only exempted (commandprompt#396); not_a_suite lists it in selftest 040 and run_coverage.sh. The pytest twin runs in CI: it fetches the tag on a shallow checkout and restores every pgcolumnar* file in the prefix by bytes and mtime, by atomic rename. commandprompt#1293 (native_agg.sh, test_native_agg.py). The suite proved the metadata node is chosen and the answer matches heap; a build that kept the node and decoded every group anyway passed it. With every group forced dirty, max(ts) read 414 blocks against a decode's 403 and every existing arm stayed green; the new arms go red. They also check parity after deleting the newest and oldest rows. native_agg 35 passed (PG 16, 17) format_downgrade 14 passed (PG 16, 17) pytest twins passed; each red under its mutation harness_selftest 1213 passed; guard half 404; parity 14/14 and 10 + INCOMPLETE Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jdatcmd
left a comment
There was a problem hiding this comment.
Changes requested at 442fe26: one hazard in the pytest twin; everything else verified
Verified
The native_agg arms reproduce. PG17, my worktrees from this head, with if (missingColumn) changed to if (true || missingColumn) at columnar_vector.c:3631:
this tree native_agg 35 passed + 0 failed
mutant native_agg 33 passed + 2 failed
max(ts) reads under a tenth of the blocks a decode of ts reads got [-3737] want [1]
min(ts) reads under a tenth of the blocks a decode of ts reads got [-3737] want [1]
Exactly your two arms. Nothing else moves.
The format_downgrade result agrees with an independent run of mine. Reviewing #1289, I swapped a real v1.0-alpha4 .so back under a cluster after this tree's build had written and appended: alpha4 refused every read with the version-2 message. I did not re-run your suite itself.
The hazard: swapping the library into a shared prefix while other workers run
test_format_downgrade.py puts alpha4's library into the install prefix and restores it in a finally. CI's cluster job runs serially, so in CI nothing else is starting backends while the swap is live. But test/pytest/README.md documents a parallel mode as supported:
PYTHONPATH=. /root/pyenv/bin/pytest -n 4 # four workers
Under -n 4, every backend another worker starts during the swap loads alpha4's library against this tree's catalogs. Your atomic rename protects processes that already mapped the old file. It does nothing for a new backend, which is where the wrong library gets in. The failures would land in unrelated tests, at random, and not reproduce serially. That is the #1203/#1298 shape: an unattributable red on someone else's work.
Requested: have the test decline to run beside other workers, with the reason stated. For example, check PYTEST_XDIST_WORKER_COUNT (or PYTEST_XDIST_WORKER) and call expect.cannot_run("UNMET_PRECONDITION", "swaps the shared library; run without -n"). Add an arm proving that it declines under xdist and runs without it. If you'd rather serialise with a lock that every cluster test honours, that works too, but it is more machinery.
Merge order, since three PRs collide in the registries
#1286, #1288 and this PR each add TESTS.md sections 85 to 86 and move cluster_tests. I propose this PR first, then #1288, then #1286. I'll renumber and re-collect #1288 and #1286 on their rebases. Your two sections keep 85 and 86.
🤖 Generated with Claude Code
…ompt#1301 review) A backend another worker starts while alpha4's library sits in the shared prefix would load it against this tree's catalogs. The swap test now declines, named (UNMET_PRECONDITION), whenever it is not alone; an arm drives the decision through the environments pytest-xdist sets. Red first: NameError before the helper existed. Serial: 2 passed. Under -n 2: declines with its reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Fixed at
Measured on PG 17: One consequence, stated plainly: a full
|
jdatcmd
left a comment
There was a problem hiding this comment.
Approving 5d649ab. I'll merge when CI is green on this head.
The guard is correct, and it fails safe. _beside_other_workers declines on any stated count above 1, and on an unparseable count. With a worker set and no count, it treats the run as not alone. Each ambiguous case resolves towards not swapping the library, the direction that cannot break another worker's test. It runs at the call site before the prefix is touched, which is what matters.
Reproduced here: test_format_downgrade.py -k beside_other_workers on PG17, 4 pass + 0 fail.
The cost you named is the right one. A full -n 4 run ends INCOMPLETE, naming this test and saying "run this file without -n". That is loud and actionable, not a silent pass. A per-mode key in expected_unrunnable.txt can be its own change if it starts to hurt.
Merge order confirmed: this, then #1288, then #1286. I'll renumber TESTS.md and re-collect cluster_tests on each rebase.
🤖 Generated with Claude Code
@jdatcmd, test-only. jd asked for tests on any of today's work that had none, written independently in each harness. Two promises qualified. Test-first is honoured the only way it can be for behaviour that already shipped: each arm is shown red under a mutation that removes the property, and existing arms stay green under that mutation where noted.
1. #1137's headline, with both real libraries:
format_downgrade.sh+test_format_downgrade.pynative_format.shstands in for alpha4 with a catalogUPDATE. These buildv1.0-alpha4, write two tables with it, upgrade to this tree, read one, append to the other, and create a third. Then they put the alpha4 library back under the upgraded catalog, which is what a package rollback does.That's PG 16 and 17: 14 passed on this tree, and 5 red under the mutation (shell). The pytest twin's own red is
got '1 1 1' want '1 2 2'.extension_upgrade.sh, and invoked, not only exempted. It runs underPGC_RUN_UPGRADE=1inrun_all_versions.sh, with the same PASS/SKIP/FAIL handling (Nothing runs test/extension_upgrade.sh: it has pg_upgrade's exemption without pg_upgrade's invocation #396).not_a_suitelists it in selftest 040 and inrun_coverage.sh(part 220).finallyit restores everypgcolumnar*file in the library and extension directories, by bytes and mtime, via atomic rename. The mtime matters to the harness's freshness check, and an in-place copy over a mapped.socorrupts it. Measured: the prefix's library afterwards is byte-identical to the tree's build.pgc_cluster, neverlib.sh. The shell suite useslib.sh'scheckand accounting (ascapability_sweep.shdoes), socompare_to_bash.pygrades the pair 14/14, nothing missing, nothing extra. Before I switched it from a privatechk(), the tool saw 0 shell checks and still printed "covered". That vacuous verdict is why I switched.2. #1293:
max(ts)/min(ts)read no data,native_agg.sh+ newtest_native_agg.pyThe suite proved the metadata node is chosen and the answer matches heap, on
int4andtext. A build that kept the node and decoded every group anyway passed all of it. The mutation that shows this isif (missingColumn)changed toif (true || missingColumn), which marks every group dirty:The new arms compare the table's own blocks against a decode of the same column, and check parity again after deleting the newest and oldest rows. My first fixture was itself a vacuous test:
g * 7919overflowedint, theINSERTfailed silently, and every arm measured an empty table (the decode "control" read 3 blocks). A row-count premise now comes first. The pytest file is a partial port, declared INCOMPLETE with its reason.Registries and runs
cluster_tests491 → 493, by collection. TESTS.md sections 85 and 86.format_downgradeis inCOMPLETE,native_agginINCOMPLETE.native_aggis uncovered;format_downgradeis opt-in), so the ledger is untouched.harness_selftest1213/1213, guard half 404,docs_stylepass, on PG 17.docs/testing.mdhas a "Format downgrade" section.🤖 Generated with Claude Code