feat: configurable compression algorithms for node compression. Fixes #16262#16261
Conversation
isubasinghe
changed the title
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: isubasinghe <isitha@pipekit.io>
Measures candidate replacements for workflow/packer's JSON+gzip+base64 node-status compression: zstd, zstd with a trained dictionary, and proto serialization, per the 2026-06-12 design spec. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: isubasinghe <isitha@pipekit.io>
Adds WORKFLOW_COMPRESSION_ALGORITHM (default: gzip) to select the compression used for status.compressedNodes. Decompression detects the algorithm from magic bytes, so readers handle both formats regardless of configuration. zstd output is ~18% smaller than gzip per the hack/compression-bench results. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: isubasinghe <isitha@pipekit.io>
-brotli-levels runs a brotli quality sweep (one codec per quality); -xz adds an xz -9 reference via the system binary (real liblzma). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: isubasinghe <isitha@pipekit.io>
Adds "brotli" as a WORKFLOW_COMPRESSION_ALGORITHM value, with WORKFLOW_COMPRESSION_BROTLI_QUALITY (default 9, the measured knee of the quality/encode-time curve: ~33% smaller than gzip, ~24ms encode at the 1MB compression threshold). Brotli streams carry no magic bytes, so the reader detects them by elimination after checking zstd and gzip magic. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: isubasinghe <isitha@pipekit.io>
Replaces WORKFLOW_COMPRESSION_BROTLI_QUALITY with WORKFLOW_COMPRESSION_LEVEL, interpreted per algorithm (gzip 1-9, zstd 1-4, brotli 0-11) and defaulting to each library's own default when unset. Documents size/speed trade-offs with synthetic benchmark numbers in offloading-large-workflows.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: isubasinghe <isitha@pipekit.io>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: isubasinghe <isitha@pipekit.io>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: isubasinghe <isitha@pipekit.io>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: isubasinghe <isitha@pipekit.io>
Adds the compression vocabulary to .spelling, moves timing units into the benchmark table header so cells are bare numbers, and relocates the internal benchmark design spec out of docs/ to live with the harness. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: isubasinghe <isitha@pipekit.io>
The xz reference shelled out to the system binary; drop it to keep the harness self-contained. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: isubasinghe <isitha@pipekit.io>
e0b1681 to
ec0e9c5
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds configurable node status compression algorithms (zstd, brotli, in addition to gzip) via ChangesConfigurable Compression Algorithm
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)flowchart TD
CompressContent -->|WORKFLOW_COMPRESSION_ALGORITHM=zstd| ZstdEncoder
CompressContent -->|WORKFLOW_COMPRESSION_ALGORITHM=brotli| BrotliWriter
CompressContent -->|default| GzipWriter
DecompressContent -->|zstd magic bytes| ZstdDecoder
DecompressContent -->|gzip magic bytes| GzipReader
DecompressContent -->|else| BrotliReader
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
util/file/fileutil.go (1)
192-206: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueZstd decode error isn't wrapped with context, unlike the brotli branch.
The brotli path wraps failures as
"failed to decompress: %w", but the zstd path returnszstdDecoder.DecodeAll(...)'s raw error directly. Minor inconsistency in error context between the two new branches.♻️ Proposed fix for consistent error wrapping
if bytes.HasPrefix(content, zstdMagic) { - return zstdDecoder.DecodeAll(content, nil) + decompressed, err := zstdDecoder.DecodeAll(content, nil) + if err != nil { + return nil, fmt.Errorf("failed to decompress: %w", err) + } + return decompressed, nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@util/file/fileutil.go` around lines 192 - 206, The DecompressContent function handles zstd, gzip, and brotli, but the zstd branch returns DecodeAll errors without the same context used in the brotli branch. Update the zstd path in DecompressContent to capture the DecodeAll error and wrap it with a consistent message like the brotli branch, so all decompression failures surface through the same error context.hack/compression-bench/DESIGN.md (1)
43-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCodec matrix table doesn't mention brotli.
Per the PR's commit history, brotli support was added after the initial benchmarking, but this table only lists
json+gzip,json+zstd(+dict), andproto+zstd(+dict). The actual implementation (codecs.go'sbrotliCodec,main.go's-brotli-levelsflag) addsjson+brotliNvariants to the matrix. Worth updating this table so the design doc stays an accurate reference for the harness.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hack/compression-bench/DESIGN.md` around lines 43 - 62, The codec matrix in DESIGN.md is missing the brotli variants, so update the table to include the json+brotliN entries added by the harness. Use the existing codec naming from brotliCodec in codecs.go and the -brotli-levels handling in main.go so the documented matrix matches the implemented compression options.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hack/compression-bench/main.go`:
- Around line 1-6: The package comment in main should point to the current
design document instead of the stale docs/superpowers/specs reference. Update
the text in the compression-bench comment so it references
hack/compression-bench/DESIGN.md, keeping the rest of the benchmark description
in sync and easy to find for readers.
- Around line 137-169: The trainDicts function builds dictionaries from the
train map by ranging over it directly, which makes the sample order
nondeterministic and can change the output between runs. Update trainDicts to
gather and sort the training entries before appending JSON and proto samples,
then pass the samples to dict.BuildZstdDict in a stable order so the dictionary
contents are reproducible.
---
Nitpick comments:
In `@hack/compression-bench/DESIGN.md`:
- Around line 43-62: The codec matrix in DESIGN.md is missing the brotli
variants, so update the table to include the json+brotliN entries added by the
harness. Use the existing codec naming from brotliCodec in codecs.go and the
-brotli-levels handling in main.go so the documented matrix matches the
implemented compression options.
In `@util/file/fileutil.go`:
- Around line 192-206: The DecompressContent function handles zstd, gzip, and
brotli, but the zstd branch returns DecodeAll errors without the same context
used in the brotli branch. Update the zstd path in DecompressContent to capture
the DecodeAll error and wrap it with a consistent message like the brotli
branch, so all decompression failures surface through the same error context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8efdeb55-ab12-468e-a5c0-195891aee691
📒 Files selected for processing (12)
.cspell.json.features/pending/configurable-node-status-compression.mddocs/environment-variables.mddocs/offloading-large-workflows.mdgo.modhack/compression-bench/DESIGN.mdhack/compression-bench/codecs.gohack/compression-bench/corpus.gohack/compression-bench/main.goutil/file/fileutil.goutil/file/fileutil_test.goworkflow/packer/packer_test.go
| // compression-bench measures candidate replacements for the node-status | ||
| // compression in workflow/packer (JSON+gzip+base64). It synthesizes node | ||
| // statuses from the examples/ specs, trains zstd dictionaries on half the | ||
| // corpus, and reports sizes/ratios/timings for each codec on the held-out | ||
| // half. See docs/superpowers/specs/2026-06-12-node-compression-benchmark-design.md. | ||
| package main |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stale design-doc reference.
This comment points to docs/superpowers/specs/2026-06-12-node-compression-benchmark-design.md, but the actual design doc in this PR lives at hack/compression-bench/DESIGN.md. Update the reference so readers can actually find it.
📝 Proposed fix
// statuses from the examples/ specs, trains zstd dictionaries on half the
// corpus, and reports sizes/ratios/timings for each codec on the held-out
-// half. See docs/superpowers/specs/2026-06-12-node-compression-benchmark-design.md.
+// half. See hack/compression-bench/DESIGN.md.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // compression-bench measures candidate replacements for the node-status | |
| // compression in workflow/packer (JSON+gzip+base64). It synthesizes node | |
| // statuses from the examples/ specs, trains zstd dictionaries on half the | |
| // corpus, and reports sizes/ratios/timings for each codec on the held-out | |
| // half. See docs/superpowers/specs/2026-06-12-node-compression-benchmark-design.md. | |
| package main | |
| // compression-bench measures candidate replacements for the node-status | |
| // compression in workflow/packer (JSON+gzip+base64). It synthesizes node | |
| // statuses from the examples/ specs, trains zstd dictionaries on half the | |
| // corpus, and reports sizes/ratios/timings for each codec on the held-out | |
| // half. See hack/compression-bench/DESIGN.md. | |
| package main |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hack/compression-bench/main.go` around lines 1 - 6, The package comment in
main should point to the current design document instead of the stale
docs/superpowers/specs reference. Update the text in the compression-bench
comment so it references hack/compression-bench/DESIGN.md, keeping the rest of
the benchmark description in sync and easy to find for readers.
| // trainDicts builds one JSON-trained and one proto-trained dictionary from | ||
| // all training blobs across every scale, matching deployment reality: a | ||
| // single shipped dictionary, whatever the workflow size. | ||
| func trainDicts(train map[int][]wfv1.Nodes, maxSize int, level zstd.EncoderLevel) ([]byte, []byte, error) { | ||
| var jsonSamples, protoSamples [][]byte | ||
| for _, blobs := range train { | ||
| for _, nodes := range blobs { | ||
| j, err := marshalJSON(nodes) | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| p, err := marshalProto(nodes) | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| // The dict builder expects many small samples (like zstd --train) | ||
| // and panics on multi-MB inputs, so chunk the blobs. | ||
| jsonSamples = append(jsonSamples, chunk(j, 64<<10)...) | ||
| protoSamples = append(protoSamples, chunk(p, 64<<10)...) | ||
| } | ||
| } | ||
| opts := dict.Options{MaxDictSize: maxSize, HashBytes: 6, ZstdLevel: level, ZstdDictID: 1} | ||
| jsonDict, err := dict.BuildZstdDict(jsonSamples, opts) | ||
| if err != nil { | ||
| return nil, nil, fmt.Errorf("train json dict: %w", err) | ||
| } | ||
| opts.ZstdDictID = 2 | ||
| protoDict, err := dict.BuildZstdDict(protoSamples, opts) | ||
| if err != nil { | ||
| return nil, nil, fmt.Errorf("train proto dict: %w", err) | ||
| } | ||
| return jsonDict, protoDict, nil | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the files and symbols involved in the comment.
git ls-files | rg '^(hack/compression-bench/(main\.go|corpus\.go)|DESIGN\.md)$'
echo '--- main.go outline ---'
ast-grep outline hack/compression-bench/main.go --view expanded || true
echo '--- corpus.go outline ---'
ast-grep outline hack/compression-bench/corpus.go --view expanded || true
echo '--- references to trainDicts / reproducibility wording ---'
rg -n 'trainDicts|identical seeds|reproduc|corpus|runs are reproducible|dict\.BuildZstdDict|eval\[scale\]' hack/compression-bench corpus.go DESIGN.md -S || trueRepository: argoproj/argo-workflows
Length of output: 3127
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant sections with line numbers.
sed -n '1,240p' hack/compression-bench/main.go
echo '--- corpus.go ---'
sed -n '1,240p' hack/compression-bench/corpus.go
echo '--- DESIGN.md matches ---'
rg -n 'identical seeds|reproduc|corpus' DESIGN.md -S || trueRepository: argoproj/argo-workflows
Length of output: 15327
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the dictionary training implementation and adjacent logic.
cat -n hack/compression-bench/main.go | sed -n '120,210p'
echo '---'
cat -n hack/compression-bench/corpus.go | sed -n '1,240p'Repository: argoproj/argo-workflows
Length of output: 12564
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where the dictionary builder may normalize input order or otherwise
# affect determinism.
rg -n 'BuildZstdDict|NewDictionary|HashBytes|ZstdDictID|sort\.Ints|range train|map\[int\]\[\]wfv1\.Nodes' -S .Repository: argoproj/argo-workflows
Length of output: 969
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant source and docs around the claim.
echo '=== hack/compression-bench/main.go (120-210) ==='
sed -n '120,210p' hack/compression-bench/main.go
echo '=== hack/compression-bench/corpus.go (1-240) ==='
sed -n '1,240p' hack/compression-bench/corpus.go
echo '=== DESIGN.md (reproducibility mentions) ==='
rg -n 'identical seeds|reproduc|determin|corpus|seed' DESIGN.md -S || true
echo '=== dictionary builder references ==='
rg -n 'BuildZstdDict|HashBytes|ZstdDictID|dict\.Options|trainDicts' -S hack/compression-bench . || trueRepository: argoproj/argo-workflows
Length of output: 12092
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '=== hack/compression-bench/main.go (120-210) ==='
sed -n '120,210p' hack/compression-bench/main.go
echo '=== hack/compression-bench/corpus.go (1-240) ==='
sed -n '1,240p' hack/compression-bench/corpus.go
echo '=== DESIGN.md (reproducibility mentions) ==='
rg -n 'identical seeds|reproduc|determin|corpus|seed' DESIGN.md -S || true
echo '=== dictionary builder references ==='
rg -n 'BuildZstdDict|HashBytes|ZstdDictID|dict\.Options|trainDicts' -S hack/compression-bench . || trueRepository: argoproj/argo-workflows
Length of output: 12092
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for the dictionary package source in the repository or module cache references.
rg -n 'package dict|func BuildZstdDict|type Options struct|ZstdDictID' -S . || true
# Show the module that provides the dict import.
rg -n 'import .*dict|github.com/.*/dict|go\.mod' -S hack/compression-bench/main.go go.mod */go.mod || trueRepository: argoproj/argo-workflows
Length of output: 584
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '=== go.mod files ==='
git ls-files 'go.mod' '**/go.mod'
echo '=== hack/compression-bench/main.go imports ==='
sed -n '1,80p' hack/compression-bench/main.goRepository: argoproj/argo-workflows
Length of output: 3395
🌐 Web query:
klauspost compress zstd BuildZstdDict sample order deterministic dictionary training
💡 Result:
The klauspost/compress library provides an experimental dictionary builder via the dict package [1][2]. The function BuildZstdDict accepts a collection of samples ([][]byte) to generate a Zstandard dictionary [1][2]. Regarding your questions on sample order and determinism: 1. Sample Order: The library documentation does not explicitly state that the order of the samples in the input slice is significant for the final dictionary structure [2][3]. However, as the builder process involves deduplication and normalization of the provided data, providing a representative, diverse set of samples is emphasized as the most important factor for dictionary quality [2][3]. 2. Deterministic Training: The BuildZstdDict function does not guarantee bit-for-bit deterministic output by default if the ZstdDictID option is set to 0, as this value triggers a random dictionary ID generation [2][3]. If a specific, non-zero ID is provided and the input sample set (including content and order) remains identical, the output should generally be deterministic; however, because the implementation is described as an experimental, high-level wrapper that performs internal processing (such as deduplication), users seeking strict reproducibility should rely on fixed inputs and avoid random ID settings [2][3][4]. The builder works by deduplicating and truncating the provided samples (using the MaxDictSize option), then constructing the dictionary using the library's internal Zstd dictionary logic [4]. It is noted to be an experimental "PoC" (Proof of Concept) that may behave differently than the official Zstandard command-line tool (zstd --train) and may struggle with very uniform or tiny datasets [2][3][4].
Citations:
- 1: https://pkg.go.dev/github.com/klauspost/compress/dict
- 2: Add dictionary builder klauspost/compress#853
- 3: https://chromium.googlesource.com/external/github.com/klauspost/compress/+/refs/tags/v1.17.1/dict/README.md
- 4: BuildZStdDict Testing klauspost/compress#950
🌐 Web query:
site:pkg.go.dev BuildZstdDict dict.Options sample order zstd dictionary training
💡 Result:
The BuildZstdDict function is part of the github.com/klauspost/compress/dict package, which provides utilities for building Zstandard compression dictionaries from samples [1]. While specific documentation detailing the fields of the Options struct for BuildZstdDict is limited in the provided search results, the dictionary training process in Zstandard generally relies on a representative set of samples to identify common patterns for improved compression ratios on small data [2][3]. The order of samples provided to dictionary training functions is generally not a defining factor for the resulting dictionary's quality, provided the sample set is representative of the data intended for future compression [2][1]. In the context of the klauspost/compress library, BuildZstdDict accepts input data and configuration via its Options structure to generate a dictionary [1]. For users of other common Go bindings for Zstandard, such as github.com/GrigoryEvko/gozstd or humungus.tedunangst.com/r/gozstd, dictionaries are typically built by passing a slice of byte slices (samples) and a desired dictionary size to a function such as BuildDict [2][4]. The resulting dictionary can then be utilized to create compression and decompression dictionaries (CDict/DDict) to optimize the compression of similar data [2].
Citations:
- 1: https://pkg.go.dev/github.com/klauspost/compress/dict
- 2: https://pkg.go.dev/github.com/GrigoryEvko/gozstd@v1.25.3-0.20250810034537-ee7d9bfba769
- 3: https://pkg.go.dev/github.com/valyala/gozstd
- 4: https://pkg.go.dev/humungus.tedunangst.com/r/gozstd
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect local Go module cache for the dict implementation, if present.
go env GOPATH GOMODCACHE 2>/dev/null || true
echo '--- candidate sources ---'
fd -a 'dict.*(go|s)$|zstd.*(go|s)$' "$(go env GOMODCACHE 2>/dev/null)" 2>/dev/null | rg 'compress|zstd|dict' || trueRepository: argoproj/argo-workflows
Length of output: 4588
🌐 Web query:
"BuildZstdDict" order samples deterministic "klauspost/compress" dictionary
💡 Result:
In the klauspost/compress library, the BuildZstdDict function does not guarantee that the order of input samples influences the dictionary creation process in a way that implies non-determinism, nor does the documentation specify a requirement for a deterministic order [1][2][3]. The BuildZstdDict function takes a collection of byte slices (samples) as input and uses them to construct a common dictionary [1][4][3]. According to the package documentation, the primary requirement is that the provided samples are representative of the input data and do not contain complete duplicates [3]. While the dictionary building process is designed to create a useful dictionary for the provided sample set [4][3], there is no information in the official documentation or source code indicating that the order of the samples in the input slice must be sorted or is otherwise strictly enforced to achieve a specific deterministic outcome [1][2]. Users are expected to supply a collection of representative samples, and the library handles the deduplication and truncation (up to the specified MaxDictSize) to build the dictionary [4][3]. If strict output reproducibility across different runs is required, it is best practice to ensure the input sample slice is ordered consistently before passing it to the function.
Citations:
- 1: Add dictionary builder klauspost/compress#853
- 2: https://chromium.googlesource.com/external/github.com/klauspost/compress/+/refs/tags/v1.17.1/dict/README.md
- 3: https://chromium.googlesource.com/external/github.com/klauspost/compress/+/refs/tags/v1.17.1/dict/
- 4: BuildZStdDict Testing klauspost/compress#950
🏁 Script executed:
#!/bin/bash
set -euo pipefail
GOMODCACHE="$(go env GOMODCACHE 2>/dev/null || true)"
echo "GOMODCACHE=$GOMODCACHE"
if [ -n "$GOMODCACHE" ] && [ -d "$GOMODCACHE" ]; then
fd -a 'dict.go' "$GOMODCACHE" | rg 'klauspost|compress|zstd' || true
fd -a 'zstd' "$GOMODCACHE" | rg 'klauspost|compress' || true
fiRepository: argoproj/argo-workflows
Length of output: 1785
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'trainDicts\(|make\(map\[int\]\[\]wfv1\.Nodes\)|train :=|eval\[scale\]|for _, scale := range scales' hack/compression-bench -SRepository: argoproj/argo-workflows
Length of output: 754
🏁 Script executed:
#!/bin/bash
set -euo pipefail
DICT_GO="/home/jailuser/go/pkg/mod/github.com/klauspost/compress@v1.18.6/zstd/dict.go"
echo "=== $DICT_GO ==="
cat -n "$DICT_GO" | sed -n '1,260p'
echo '--- relevant symbols ---'
rg -n 'func BuildZstdDict|type Options struct|sort\.|range .*samples|dedup|truncate|hash' "$DICT_GO" -n -SRepository: argoproj/argo-workflows
Length of output: 8171
🏁 Script executed:
#!/bin/bash
set -euo pipefail
README="/home/jailuser/go/pkg/mod/github.com/klauspost/compress@v1.18.6/zstd/dict/README.md"
echo "=== $README ==="
cat -n "$README" | sed -n '1,260p'
echo '--- order / reproducibility notes ---'
rg -n 'order|determin|reproduc|duplicate|sample' "$README" -n -SRepository: argoproj/argo-workflows
Length of output: 369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
DICT_GO="/home/jailuser/go/pkg/mod/github.com/klauspost/compress@v1.18.6/zstd/dict.go"
# Show just the training/building path.
rg -n 'func BuildZstdDict|func .*Dict|samples|MaxDictSize|ZstdDictID|HashBytes|dedup|trunc' "$DICT_GO" -A 120 -B 20 -n -SRepository: argoproj/argo-workflows
Length of output: 10767
🏁 Script executed:
#!/bin/bash
set -euo pipefail
DICT_GO="/home/jailuser/go/pkg/mod/github.com/klauspost/compress@v1.18.6/zstd/dict.go"
README="/home/jailuser/go/pkg/mod/github.com/klauspost/compress@v1.18.6/zstd/dict/README.md"
echo '=== BuildZstdDict implementation excerpt ==='
sed -n '1,260p' "$DICT_GO"
echo '=== dict README excerpt ==='
sed -n '1,240p' "$README"Repository: argoproj/argo-workflows
Length of output: 6388
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
dict_go = Path('/home/jailuser/go/pkg/mod/github.com/klauspost/compress@v1.18.6/zstd/dict.go')
text = dict_go.read_text()
for key in ['func BuildZstdDict', 'sort.', 'range samples', 'dedup', 'MaxDictSize', 'ZstdDictID', 'HashBytes']:
print(f'## {key}')
idx = text.find(key)
print('found' if idx >= 0 else 'not found', idx)
PYRepository: argoproj/argo-workflows
Length of output: 349
🏁 Script executed:
#!/bin/bash
set -euo pipefail
DICT_GO="/home/jailuser/go/pkg/mod/github.com/klauspost/compress@v1.18.6/zstd/dict.go"
# Focus on the builder and any preprocessing it does to samples.
rg -n 'func BuildZstdDict|func .*build|sample|samples|dedup|sort\.|MaxDictSize|ZstdDictID|HashBytes' "$DICT_GO" -A 40 -B 20 -n -SRepository: argoproj/argo-workflows
Length of output: 1691
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate BuildZstdDict in the installed klauspost/compress module and inspect it.
MODDIR="/home/jailuser/go/pkg/mod/github.com/klauspost/compress@v1.18.6"
rg -n 'BuildZstdDict|BuildDictOptions|BuildDict\(' "$MODDIR" -SRepository: argoproj/argo-workflows
Length of output: 1117
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the exact BuildZstdDict implementation, if present.
MODDIR="/home/jailuser/go/pkg/mod/github.com/klauspost/compress@v1.18.6"
for f in "$MODDIR"/**/*.go; do :; done 2>/dev/null || true
rg -n 'func BuildZstdDict|type Options struct|BuildZstdDict\(' "$MODDIR" -A 120 -B 20 -SRepository: argoproj/argo-workflows
Length of output: 27502
🏁 Script executed:
#!/bin/bash
set -euo pipefail
MODDIR="/home/jailuser/go/pkg/mod/github.com/klauspost/compress@v1.18.6"
echo '--- symbols ---'
rg -n 'BuildZstdDict|BuildDictOptions|func BuildDict|type Options struct' "$MODDIR" -S
echo '--- files containing BuildZstdDict ---'
rg -l 'BuildZstdDict' "$MODDIR" -SRepository: argoproj/argo-workflows
Length of output: 1527
Sort the training samples before building the dictionaries. dict.BuildZstdDict processes the input slice in order and uses sample position in its statistics, so ranging over train directly can change the resulting dictionary between runs even with the same seed. hack/compression-bench/main.go:140-164
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hack/compression-bench/main.go` around lines 137 - 169, The trainDicts
function builds dictionaries from the train map by ranging over it directly,
which makes the sample order nondeterministic and can change the output between
runs. Update trainDicts to gather and sort the training entries before appending
JSON and proto samples, then pass the samples to dict.BuildZstdDict in a stable
order so the dictionary contents are reproducible.
| ctx := logging.TestContext(t.Context()) | ||
| content := "{\"pod-limits-rrdm8-591645159\":{\"id\":\"pod-limits-rrdm8-591645159\",\"phase\":\"Succeeded\"}}" | ||
|
|
||
| var compressed string |
There was a problem hiding this comment.
TestZstdCompression and TestBrotliCompression share this compressed variable across subtests: the first subtest writes it, later subtests read it. Running a later subtest in isolation (e.g. go test -run 'TestZstdCompression/DecompressDetectsZstdWithoutEnvVar') leaves it empty and the test fails — verified locally. The subtests depend on source-order execution and aren't independently runnable/debuggable. Consider compressing fresh inside each subtest (or a helper) so each stands alone.
|
|
||
| // TestCompressionLevel ensures WORKFLOW_COMPRESSION_LEVEL applies to each | ||
| // algorithm with algorithm-specific ranges. | ||
| func TestCompressionLevel(t *testing.T) { |
There was a problem hiding this comment.
These level tests (and TestBrotliCompression's InvalidLevelFallsBackToDefault/ExplicitLevel) only assert that compress→decompress round-trips — which passes regardless of which level was actually used. A regression that ignored WORKFLOW_COMPRESSION_LEVEL entirely would still go green, so the compressionLevel() parsing/validation path is effectively unverified. Observing an effect of the level would fix this, e.g. assert that max-level output is smaller than (or differs from) min-level output for the same input, or that invalid-level output is byte-identical to unset-level output.
| // variable while the reader detects the algorithm from the content alone. | ||
| func TestZstdCompression(t *testing.T) { | ||
| ctx := logging.TestContext(t.Context()) | ||
| content := "{\"pod-limits-rrdm8-591645159\":{\"id\":\"pod-limits-rrdm8-591645159\",\"phase\":\"Succeeded\"}}" |
There was a problem hiding this comment.
Nit: this 90-character JSON fixture (and the ctx := logging.TestContext(t.Context()) setup) is duplicated verbatim in all three new test functions. A package-level const and/or a small round-trip helper would remove the repetition.
| // by `CompressionAlgorithmEnvVarKey` (gzip by default). DecompressContent | ||
| // detects the algorithm from the content, so the variable only affects writes. | ||
| func CompressContent(ctx context.Context, content []byte) []byte { | ||
| switch env.GetString(CompressionAlgorithmEnvVarKey, GZipAlgorithm) { |
There was a problem hiding this comment.
An unrecognized WORKFLOW_COMPRESSION_ALGORITHM value (typo, or wrong case like ZSTD — the comparison is case-sensitive) silently falls through to gzip with no log line, whereas an invalid WORKFLOW_COMPRESSION_LEVEL warns via compressionLevel(). An operator who sets the algorithm and fat-fingers it gets no feedback that their setting was ignored. Suggest logging a warning in a default: case for parity.
| // compression in workflow/packer (JSON+gzip+base64). It synthesizes node | ||
| // statuses from the examples/ specs, trains zstd dictionaries on half the | ||
| // corpus, and reports sizes/ratios/timings for each codec on the held-out | ||
| // half. See docs/superpowers/specs/2026-06-12-node-compression-benchmark-design.md. |
There was a problem hiding this comment.
This references docs/superpowers/specs/2026-06-12-node-compression-benchmark-design.md, which doesn't exist in the repo — the design doc actually ships as hack/compression-bench/DESIGN.md in this PR. Looks like a leftover pointer to local working notes; suggest pointing at DESIGN.md or dropping the sentence.
|
|
||
| Measure whether replacing the current node-status compression (JSON → gzip → | ||
| base64, `workflow/packer`) with proto serialization and/or zstd with a trained | ||
| dictionary is worth a format migration. This is a measurement prototype only: |
There was a problem hiding this comment.
Should this harness ship in the PR at all? Its own design doc describes it as "a measurement prototype only" whose numbers "decide whether an integration phase happens later" — and that decision is now made (the results are baked into docs/offloading-large-workflows.md). As committed it: benchmarks codecs the feature doesn't ship (proto encoding, zstd dictionaries), is the sole user of klauspost/compress/dict, isn't wired into the Makefile or CI (unlike the other hack/ tools, per hack/README.md), yet must compile and pass lint forever via go build ./.... Consider dropping it (keeping the measured results in the docs) or moving it out of the mainline tree.
|
|
||
| Large workflow node statuses can now be compressed with `zstd` or `brotli` instead of gzip via the `WORKFLOW_COMPRESSION_ALGORITHM` environment variable, with `WORKFLOW_COMPRESSION_LEVEL` tuning the level. | ||
| Decompression auto-detects the algorithm. | ||
| See [node status compression](../docs/offloading-large-workflows.md#node-status-compression). |
There was a problem hiding this comment.
This link will break when rendered: hack/featuregen copies the feature body verbatim into docs/new-features.md, and from there ../docs/offloading-large-workflows.md escapes the mkdocs docs root (mkdocs runs with strict: true). Should be the bare form used everywhere else in docs/ — offloading-large-workflows.md#node-status-compression — matching this PR's own environment-variables.md.
| assert.NotEmpty(t, wf.Status.Nodes) | ||
| assert.Empty(t, wf.Status.CompressedNodes) | ||
| }) | ||
| t.Run("LargeWorkflowZstd", func(t *testing.T) { |
There was a problem hiding this comment.
LargeWorkflowZstd and LargeWorkflowBrotli are line-for-line copies of the existing LargeWorkflow subtest, differing only by the t.Setenv line — three copies of the same ~17-line body. A table-driven subtest over {"", file.ZStdAlgorithm, file.BrotliAlgorithm} would cover all three with one body.
Signed-off-by: isubasinghe <isitha@pipekit.io>
2 participants
Fixes #16262
Motivation
Node statuses are gzip-compressed into
/status/compressedNodes. In my benchmarks zstd is ~12–18% smaller at the same speed, brotli ~28–41% smaller for more CPU, which stretches how far you get before needing offloading. Switching the default would break downgrades, so this just makes it configurable; gzip stays the default.Modifications
WORKFLOW_COMPRESSION_ALGORITHM(gzip/zstd/brotli) andWORKFLOW_COMPRESSION_LEVELenv vars inutil/file. Invalid values warn and fall back.klauspost/compressandandybalholm/brotlipromoted from indirect to direct.hack/compression-bench: the harness behind the numbers. It also ruled out zstd dictionaries and proto encoding.Verification
Unit tests in
util/file(round-trips, detection, fallbacks) and end-to-end zstd/brotli tests inworkflow/packer. The harness round-trip-verifies every codec.Documentation
New section in
docs/offloading-large-workflows.mdwith the benchmark table and a downgrade warning. Env vars listed indocs/environment-variables.md. Feature file added for release notes.AI
Claude Code helped with the code, tests, docs, and this description. Reviewed and benchmarked by me.
Summary by CodeRabbit
New Features
gzip,zstd, orbrotli, with configurable compression level settings.Bug Fixes
Documentation