Feat/streaming op log: Implements streaming zst writer for tsv operation logs#5
Merged
Conversation
StreamingOpsWriter writes Operations to a zstd-compressed TSV file as they arrive via a channel, without ever accumulating them in a slice. Memory overhead is constant (~1.5 MB) regardless of benchmark duration or concurrency, compared to the current batch design that grows O(n). Key design points: - Channel buffer matches collector.rcv (1000) for back-pressure parity - clientID stamped per-row in goroutine (ops arrive with empty ClientID) - Empty clientID preserves any ClientID already set on the Operation - Two close patterns: Close() for standalone/test use; Wait() for use after a Collector has closed the channel (avoids double-close) - 256 KiB bufio.Writer wrapping zstd SpeedBetterCompression encoder - 9 unit tests covering: file creation, op count, clientID stamping, empty-clientID passthrough, zero-op run, field preservation, collector-integrated Wait() pattern, and concurrent-send race check All tests pass with -race.
addCollector now accepts optional fullExtra ...chan<- bench.Operation.
When --full is set and a streaming channel is provided, ops are fanned
out to that channel via NewNullCollector (no in-memory accumulation).
When no extra channel is provided and --full is set, the existing batch
mode (NewOpsCollector) is used unchanged — preserving backward
compatibility for the distributed agent code path.
New / updated tests (cli/addcollector_test.go):
Test 8: streaming mode ops go to channel, not memory
Test 9: streaming mode live collector still receives every op
Test 10: batch fallback when no streaming channel is given
Test 11: end-to-end — StreamingOpsWriter wired through addCollector
writes correct csv.zst file with clientID stamped
All 11 addCollector tests pass.
Changes to cli/benchmark.go (local/standalone benchmark path): - Compute fileName and cID before addCollector so the streaming writer can be created and wired into the collector fan-out before b.Start(). - When --full is set, create a StreamingOpsWriter immediately; the output file exists on disk from that point (partial record available on interrupt). - Pass csvWriter.Receiver() to addCollector as a fullExtra channel. addCollector uses NewNullCollector (no in-memory accumulation) when a streaming channel is provided. - After Collector.Close() (which closes the writer channel), call csvWriter.Wait() to block until the goroutine has flushed to disk. - The existing batch path (if ops := retrieveOps(); len(ops) > 0) still handles the non-streaming --full case and the non-full case unchanged. In streaming mode retrieveOps returns empty, so execution falls through to the else-if branch that writes json.zst from the live aggregate. The distributed agent path (runClientBenchmark) retains batch mode; it calls addCollector with no extra channel, which keeps using NewOpsCollector. This is a deferred improvement. README.md: update --full memory note to reflect constant ~1.5 MB overhead rather than the previous O(n) batch behaviour.
85ae59d to
c5999e7
Compare
Capture rcv channel by value before launching the goroutine so the goroutine closure doesn't access c.rcv via a field read. Without this, Close() can write c.rcv = nil (under mu) while the goroutine is still reading c.rcv to pass to Live() — a data race detected by -race. Fixes: TestAddCollector_DefaultMode_UpdatesChannelFunctional failing under go test -race ./...
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.
PR: bench: stream --full ops to csv.zst without in-memory accumulation
Branch:
russfellows:feat/streaming-op-log→minio:masterDepends on: PR minio#475 (
russfellows:fix/default-tsv-output) — this branch is stacked on topAuthor: Russ Fellows
Date: April 2026
Summary
When
--fullis used, warp currently accumulates everyOperationstruct in memoryfor the entire benchmark duration, then writes them all to
<benchdata>.csv.zstin asingle post-benchmark flush. Memory scales super-linearly with duration × concurrency:
a 1-hour / 32-concurrent run at 4 KiB objects projects to ~8 GB of extra heap.
This PR replaces that batch-write path with a background goroutine that streams
each operation to the zstd encoder as it completes, making
--fullmemory overheadconstant at ~1.5 MB regardless of how long the benchmark runs or how many workers
are active.
The output file format and all downstream tooling (
warp analyze --full,polarWarp, custom log parsers) are
completely unchanged — the on-disk bytes are identical to the batch path.
Problem in Detail
The existing path (simplified):
Measured memory overhead per op grows as Go's GC holds the entire live slice:
Solution
New type:
pkg/bench/StreamingOpsWriterNewStreamingOpsWriter(path, clientID, cmdLine)— creates the output fileimmediately (before the benchmark starts), writes the TSV header, and starts a
background goroutine.
Receiver() chan<- Operation— write-only channel passed as anextratoNewNullCollector; the collector fan-outs every arriving op into it.Wait()— called afterCollector.Close()(which closes the channel);blocks until the goroutine has flushed and finalized the zstd frame.
Close()— standalone / test use only: closes the channel then waits.The background goroutine uses a 256 KiB
bufio.Writerwrapping azstd.SpeedBetterCompressionencoder. On any write error it drains the channelto avoid blocking benchmark goroutines, and surfaces the error via
Wait().addCollectorrefactor (cli/benchmark.go)addCollectornow accepts variadicfullExtra ...chan<- bench.Operationanddispatches into three modes:
retrieveOps--discard-outputNewNullCollector--full+fullExtraprovidedNewNullCollector(extras+live+fullExtra)--full, nofullExtraNewOpsCollector(extras+live)runBenchwiring (cli/benchmark.go)In the local (non-distributed)
--fullpath:StreamingOpsWriteris created beforeaddCollector— the output fileexists from the first moment of the benchmark.
csvWriter.Receiver()is passed asfullExtratoaddCollector.c.Collector.Close(),csvWriter.Wait()flushes the final zstd frame.retrieveOps()now returns nil in streaming mode, the existingelse if updates != nilbranch naturally handles thejson.zstaggregatefile — both outputs are produced as before.
The distributed path (
runClientBenchmark) is unchanged and continues to usethe batch collector.
Files Changed
pkg/bench/streaming_writer.goStreamingOpsWriterimplementation (153 lines)pkg/bench/streaming_writer_test.go-race(368 lines)cli/benchmark.goaddCollectorsignature + streaming wiring inrunBench(+71 / -14)cli/addcollector_test.goREADME.md--fullmemory note to reflect constant overheaddocs/Warp-streaming-log-Design.mddocs/issue-streaming-full-writer.mddocs/RNG_ANALYSIS.mddocs/.gitignore.envNet: +1,539 lines inserted, −29 deleted across 9 files.
Tests
Unit tests —
pkg/bench(9 tests, all pass with-race)TestStreamingOpsWriter_FileCreatedOnNewTestStreamingOpsWriter_OpCountTestStreamingOpsWriter_ClientIDStampedclientIDoverwrites every rowTestStreamingOpsWriter_EmptyClientID_PreservesFieldclientIDpreserves op's existing fieldTestStreamingOpsWriter_EmptyRunTestStreamingOpsWriter_ValuesPreservedTestStreamingOpsWriter_MultipleOps_AllClientIDSetTestStreamingOpsWriter_Wait_CollectorPatternWait()returns cleanTestStreamingOpsWriter_ConcurrentSendsIntegration tests —
cli(4 new tests inaddcollector_test.go)TestAddCollector_StreamingMode_OpsGoToChannelTestAddCollector_StreamingMode_LiveCollectorAlsoReceivesTestAddCollector_FullMode_BatchFallback_NoChannelTestAddCollector_Streaming_EndToEndaddCollector+StreamingOpsWriter→csv.zst→ parse + verify clientIDAll 20 tests (
11cli +9bench + generator tests) pass withgo test ./...andwith
go test -race ./cli/... ./pkg/bench/....Integration / End-to-End Validation
Tested against a live MinIO target (TLS, 4-node):
All output files were verified with:
warp analyze --full <file>.csv.zst— correct op counts and throughputwarp analyze <file>.csv.zst— re-aggregate path unchangedwarp analyze <file>.json.zst— aggregate JSON output still produced alongsidecorrect per-op-type latency percentiles and throughput buckets
Memory Overhead (Before vs After)
Before (batch path):
ops_per_second × durationAfter (streaming path):
bufio.Writer+ zstd encoder internal buffersBackward Compatibility
--fullflag semantics are unchanged from the user's perspectivewarp analyzesubcommands work without modification--warp-client) is untouchedRelationship to PR minio#475
This PR is stacked on
fix/default-tsv-output(PR minio#475), which restored the--fullper-transaction.csv.zstoutput that was inadvertently removed in aprevious refactor. This PR then replaces that restored batch write with the
streaming implementation. If PR minio#475 lands first, this PR can be rebased directly
onto
masterwith no conflicts.