Skip to content

fix: SIGPIPE false negatives in pipefail shell tests - #3643

Merged
mkoura merged 2 commits into
masterfrom
fix/pipefail_sigpipe_grep
Aug 20, 2026
Merged

fix: SIGPIPE false negatives in pipefail shell tests#3643
mkoura merged 2 commits into
masterfrom
fix/pipefail_sigpipe_grep

Conversation

@mkoura

@mkoura mkoura commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Problem

A regression run wedged with 8 of 9 cluster instances stuck in
respin_in_progress, each start-cluster burning its full 7200s
tx-firehose wait and then retrying, forever - while tx-firehose was
demonstrably submitting transactions the whole time.

The wait was:

if tail -n 100 "$logfile" | grep -q "TxFirehose.Submit.Success"; then

grep -q exits on the first match and closes the pipe while tail may
still have a pending write(). tail dies on SIGPIPE with 141, and
pipefail promotes that to the pipeline's exit status, so the test reads
as "no match" even though the pattern is right there. Reproduced against
the live log:

$ tail -n 100 .../tx-firehose.stderr | wc -c
21691
$ bash -c 'set -euo pipefail; if tail -n 100 .../tx-firehose.stderr | grep -q "TxFirehose.Submit.Success"; then echo MATCH; else echo "NOMATCH rc=$?"; fi'
NOMATCH rc=141

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
tail with the most left to write. The healthier the tool, the likelier
the 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_*_tx helpers, plus check_spend_success, which had the
inverted failure mode - 141 negated by ! returned "inputs spent" while
they 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 a
    while loop, so 141 ended it early and skipped waiting for the
    relay node to finish replaying
    , silently. Dropping -q is enough:
    plain grep scans all of its input, so tail always finishes
    writing. Verified against /bin/grep (GNU grep 3.11) rather than the
    shell's ugrep shim, in case of a /dev/null-implies--q
    optimisation - there is none.
  • runner/runc.sh - a single awk replaces sed | head, so the
    assignment sees awk's own status and an unreadable .git still aborts
    under errexit.
  • scripts/postgres-start.sh - drops the pipeline entirely. A quoted
    [[ == ]] right-hand side is already a literal match, which is all
    grep -F was there for, so this loses a fork too.

Verification

  • Every changed predicate exercised across match / no-match / producer-
    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 to sed | head on this worktree's real
    .git, on a two-gitdir file, and on a file with no gitdir: line.
  • postgres-start.sh: matches old behaviour on all cases including
    pg_dir=/var/tmp/pg*x, where * stays literal and /var/tmp/pgYx
    correctly does not match.
  • ./ai_run.sh make lint clean.
  • cardonnay v0.4.4 is exactly v0.4.3 plus the one commit.

mkoura added 2 commits August 20, 2026 22:38
`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.
@mkoura
mkoura requested a review from saratomaz as a code owner August 20, 2026 20:40
@mkoura
mkoura merged commit b35655c into master Aug 20, 2026
2 checks passed
@mkoura
mkoura deleted the fix/pipefail_sigpipe_grep branch August 20, 2026 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant