Write list text output to os.Stdout in a single write - #1572
Merged
Merged
Conversation
PrintBackup text format rendered the tabwriter straight onto os.Stdout.
tabwriter.Flush emits every cell and every padding run as a separate
Write (11 writes per row), while zerolog writes to stderr. Under
`docker exec` the daemon reads stdout and stderr from separate pipes
with no ordering guarantee, so under load a stderr line lands between
two cells of one row:
empty_key_prefix_1_22_32026-09-18 10:50:10.499 WRN ... skip nameless entry
2026-09-18 10:50:05 remote ...
This failed TestEmptyKeyPrefixRetention on CI (`(?m)^<name> ` no longer
matched). Reproduced locally with 8 parallel `docker exec` loops of a
program doing the same stderr line + chunked stdout row: 1 spliced
output in 1200 runs; the buffered single-write variant: 0 in 1200.
Render the table into a bytes.Buffer and hand it to os.Stdout in one
Write, same as 4a22d04 did for the machine-readable formats.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
printLiveTableRows and renderTextSection had the same tabwriter over os.Stdout as PrintBackup, so a stderr log line could split a row of `tables` / `list tables` output under docker exec. Move all three onto newStdoutTabWriter, which renders into a buffer and writes the table to os.Stdout in one Write. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Coverage Report for CI Build 35420933277Coverage at 67.559% (no base build to compare)Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
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.
Fixes the
Test (1.27, 22.3)failure of https://github.com/Altinity/clickhouse-backup/actions/runs/35335959574 (TestEmptyKeyPrefixRetention).Root cause
PrintBackupin text format renders the tabwriter directly ontoos.Stdout.tabwriter.Flushemits every cell and padding run as a separateWrite(11 writes per row), zerolog writes to stderr, and underdocker execthe daemon copies stdout and stderr from separate pipes with no ordering guarantee. Under CI load a stderr log line landed between the first two cells of a row:so
(?m)^empty_key_prefix_1_22_3no longer matched. The splice point is exactly the boundary between tabwriter's write #1 (the name) and write #2 (the padding).Reproduced locally with 8 parallel
docker execloops of a tiny program doing the same stderr line + chunked stdout row: 1 spliced output in 1200 runs. With the table rendered into a buffer and written once: 0 in 1200.Fix
Render the table into a
bytes.Bufferand write it toos.Stdoutin oneWrite(newStdoutTabWriter), the same approach 4a22d04 applied tojson/yaml/csv/tsvin v2.8.1. The second commit applies it to the two other tabwriter sites inlist.go(printLiveTableRows,renderTextSectionbehindtables/list tables), which had the same defect.Verification
GOFLAGS= go test ./pkg/backup/: 255 passedCLICKHOUSE_VERSION=22.3 RUN_TESTS='^(TestTablesCommand|TestTablesCommandListParts|TestListFormat|TestEmptyKeyPrefixRetention)$' ./test/integration/run.sh: PASSThe second failed job of that run,
Test (1.27, 26.3), is unrelated: all integration tests passed and theReport integration coveragestep failed because the Coveralls release download returned504 Gateway Time-out.🤖 Generated with Claude Code