Skip to content

Write list text output to os.Stdout in a single write - #1572

Merged
Slach merged 2 commits into
masterfrom
list-text-single-write
Sep 22, 2026
Merged

Slach merged 2 commits into
masterfrom
list-text-single-write

Conversation

@Slach

@Slach Slach commented Sep 19, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes the Test (1.27, 22.3) failure of https://github.com/Altinity/clickhouse-backup/actions/runs/35335959574 (TestEmptyKeyPrefixRetention).

Root cause

PrintBackup in text format renders the tabwriter directly onto os.Stdout. tabwriter.Flush emits every cell and padding run as a separate Write (11 writes per row), zerolog writes to stderr, and under docker exec the 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:

empty_key_prefix_1_22_32026-09-18 10:50:10.499 WRN pkg/storage/general.go:383 > BackupList: skip nameless entry "/" ...
   2026-09-18 10:50:05   remote      all:8.60KiB,...   tar, regular

so (?m)^empty_key_prefix_1_22_3 no 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 exec loops 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.Buffer and write it to os.Stdout in one Write (newStdoutTabWriter), the same approach 4a22d04 applied to json/yaml/csv/tsv in v2.8.1. The second commit applies it to the two other tabwriter sites in list.go (printLiveTableRows, renderTextSection behind tables / list tables), which had the same defect.

Verification

  • GOFLAGS= go test ./pkg/backup/: 255 passed
  • CLICKHOUSE_VERSION=22.3 RUN_TESTS='^(TestTablesCommand|TestTablesCommandListParts|TestListFormat|TestEmptyKeyPrefixRetention)$' ./test/integration/run.sh: PASS

The second failed job of that run, Test (1.27, 26.3), is unrelated: all integration tests passed and the Report integration coverage step failed because the Coveralls release download returned 504 Gateway Time-out.

🤖 Generated with Claude Code

Slach and others added 2 commits September 19, 2026 09:10
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>
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 35420933277

Coverage at 67.559% (no base build to compare)

Details

  • Coverage remained the same as the base build.
  • Patch coverage: 5 uncovered changes across 1 file (14 of 19 lines covered, 73.68%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
pkg/backup/list.go 19 14 73.68%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 27086
Covered Lines: 18299
Line Coverage: 67.56%
Coverage Strength: 68815.97 hits per line

💛 - Coveralls

@Slach
Slach merged commit 0ae1c95 into master Sep 22, 2026
114 of 120 checks passed
@Slach
Slach deleted the list-text-single-write branch September 22, 2026 17:34
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.

2 participants