fix: re-check topology per sample, quote --useafter, unbuffer stdout - #10
Open
lesandie wants to merge 2 commits into
Open
fix: re-check topology per sample, quote --useafter, unbuffer stdout#10lesandie wants to merge 2 commits into
lesandie wants to merge 2 commits into
Conversation
Clears the three hardening items left over from the deletion-scope audit. Topology is re-checked before every sample rather than once per run. The preflight was a point-in-time check while the anti-join loop can run for hours; a replica that dropped out mid-run took its references with it, so blobs it alone held started looking orphaned and nothing noticed. ch_client is free at that point -- the previous sample's stream has closed. Removing both preflight call sites now fails two tests; removing either one alone is covered by the other, so the old M7 mutation is obsolete rather than surviving. --useafter is quoted as a SQL string literal. It was interpolated bare, so an operator-supplied value landed as an identifier -- the only unquoted value in the anti-join WHERE clause. The strict xfail recorded for it flipped to XPASS and became a real test, which is what strict xfail is for. The image sets PYTHONUNBUFFERED=1. stdout is a pipe under Kubernetes so print() was block-buffered, and Python's default SIGTERM handling exits without flushing: a Job killed at activeDeadlineSeconds lost its buffered tail, including the closing "s3gc: OK", and the dev-automation shell echoes interleaved wrongly against it. Logger records were never affected -- StreamHandler.emit() flushes per record. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Since the per-sample topology re-check landed, do_use() has two preflight_cluster() call sites and the top-level one is redundant. That makes the M7 mutation survive the suite -- removing either site alone is covered by the other. Not a defect: removing both still fails two tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
1. Topology re-checked per sample (delete path)
The preflight was point-in-time, run once, while the anti-join loop can run for hours. A replica dropping out mid-run takes its references with it, so blobs it alone holds start looking orphaned — and nothing noticed. Now re-verified before each sample, failing closed.
ch_clientis free at that point: the previous sample's stream has closed, so this does not reintroduceSESSION_IS_LOCKED.Cost is one
clusterAllReplicas(cluster, system.one)count per sample (4 by default).Note on the mutation suite: M7 ("remove the preflight call") now survives, because it removes only the top-level call and the per-sample check still covers it. That is redundancy, not a regression — removing both call sites fails
test_delete_runs_preflight_before_touching_s3andtest_topology_is_rechecked_for_every_sample. Verified explicitly.2.
--useafterquotedInterpolated bare, so the value landed as a SQL identifier — the only unquoted value in the anti-join
WHERE. The strictxfailfrom #6 flipped to XPASS and became a real test, which is what strict xfail is for. Addedtest_useafter_escapes_quotesfor embedded quotes.3.
PYTHONUNBUFFERED=1in the imagestdout is a pipe under Kubernetes so
print()was block-buffered, and Python's default SIGTERM handling exits without flushing. A Job killed atactiveDeadlineSecondslost its buffered tail including the closings3gc: OK, and thedev-automationshell echoes interleaved wrongly against it. Logger records were never affected —StreamHandler.emit()flushes per record.Checks
TODO.mdis down to the three items that predate this work.🤖 Generated with Claude Code