fix: SIGPIPE false negatives in pipefail shell tests - #3643
Merged
Conversation
`grep -q` and `head` exit on first match, closing the pipe while the producer may still have a pending write. The producer dies on SIGPIPE with 141, and `pipefail` promotes that to the pipeline's exit status, so the test reads as "no match" even when the pattern is present. It is a race on syscall interleaving, not a size threshold, so it fires intermittently and the odds worsen as the scanned output grows. - testnets/start-cluster: the replay wait is a `while` loop, so 141 ended it early and skipped waiting for the relay node to finish replaying. Dropping `-q` is enough -- plain `grep` scans all of its input, so `tail` always finishes writing. - runc.sh: a single `awk` replaces `sed | head`. One process, so its own status is what the assignment sees. - postgres-start.sh: drop the pipeline entirely; a quoted `[[ == ]]` right-hand side is already a literal match, which is all `grep -F` was there for.
Carries the pipefail SIGPIPE fix in the cluster start scripts. Without it `_wait_for_tx_firehose_tx` cannot see a healthy tx-firehose, so cluster start burns its full 7200s timeout and retries indefinitely.
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.
Problem
A regression run wedged with 8 of 9 cluster instances stuck in
respin_in_progress, eachstart-clusterburning its full 7200stx-firehosewait and then retrying, forever - while tx-firehose wasdemonstrably submitting transactions the whole time.
The wait was:
grep -qexits on the first match and closes the pipe whiletailmaystill have a pending
write().taildies on SIGPIPE with 141, andpipefailpromotes that to the pipeline's exit status, so the test readsas "no match" even though the pattern is right there. Reproduced against
the live log:
The perverse part: a healthy tx tool fills the last 100 log lines with
the very pattern being matched, so the match lands on line 1, leaving
tailwith the most left to write. The healthier the tool, the likelierthe false negative.
This is a race on syscall interleaving, not a size threshold - the same
construct on the same file returned 141 and later 0. Output size only
shifts the odds.
Changes
Most of the affected code is in cardonnay, fixed there and released as
0.4.4 (IntersectMBO/cardonnay#160): five sites including all three
_wait_for_tx_*_txhelpers, pluscheck_spend_success, which had theinverted failure mode - 141 negated by
!returned "inputs spent" whilethey were not. This PR bumps the dependency to pull that in.
Three sites in this repo:
cluster_scripts/testnets/start-cluster- the replay wait is awhileloop, so 141 ended it early and skipped waiting for therelay node to finish replaying, silently. Dropping
-qis enough:plain
grepscans all of its input, sotailalways finisheswriting. Verified against
/bin/grep(GNU grep 3.11) rather than theshell's
ugrepshim, in case of a/dev/null-implies--qoptimisation - there is none.
runner/runc.sh- a singleawkreplacessed | head, so theassignment sees awk's own status and an unreadable
.gitstill abortsunder
errexit.scripts/postgres-start.sh- drops the pipeline entirely. A quoted[[ == ]]right-hand side is already a literal match, which is allgrep -Fwas there for, so this loses a fork too.Verification
failure / large-input-with-early-match; the replay wait is 0/15 false
negatives on a 2.4MB input where the old form was 10/10.
runc.sh: output identical tosed | headon this worktree's real.git, on a two-gitdirfile, and on a file with nogitdir:line.postgres-start.sh: matches old behaviour on all cases includingpg_dir=/var/tmp/pg*x, where*stays literal and/var/tmp/pgYxcorrectly does not match.
./ai_run.sh make lintclean.v0.4.4is exactlyv0.4.3plus the one commit.