JITSU-163: klauspost/gzip on the warehouse batch path, with the level change by sahiltyagi-jitsu · Pull Request #1506 · jitsucom/jitsu

sahiltyagi-jitsu · 2026-09-09T12:55:20Z

Third of the small PRs for JITSU-163, covering the warehouse batch path: the JSON, JSONArray and CSV marshallers, and the transactional SQL stream.

Stacked on #1504 (itself on #1503) — merge in order. No go.mod change; bulkerlib already carries klauspost as a direct dependency from #1503.

The level change is part of this, deliberately

The library swap and the compression level cannot be separated. Per the ticket's own benchmarks:

Codec	Output
`compress/gzip` L4 — what we run today	185.5 MB
`klauspost/gzip` L4	203.2 MB
`klauspost/gzip` L6	177.4 MB

Swapping the library while keeping level 4 would emit 9.5% larger files than today — the ticket says as much: "klauspost L4 buys its speed with 9.5% larger output — don't take it." Level 6 is the equivalent of the stdlib level 4 it replaces: 4.4% smaller and 25% cheaper.

I originally planned these as two PRs for separate revert boundaries, which was wrong — it would have allowed the regression to merge on its own. The level now lives in a named gzipLevel constant with the reasoning inline, so it doesn't get "optimised" back down.

Which files, and why these

The ticket scopes this as "all five warehouse adapters", but snowflake.go, redshift.go, clickhouse.go, bigquery.go and the Mixpanel adapter contain no gzip calls. GetBatchFileCompression() only returns a declared value (sql_adapter_base.go:166), and redshift's gzip at line 34 is the COPY … csv gzip SQL keyword in a template string. The compression happens where a marshaller is constructed, so the set closes on marshaller.go and sql/abstract_transactional.go. Happy to narrow it if you meant something else.

Verification

bulkerlib/implementations/sql passes against real database containers — it writes, compresses and loads batch files, which is exactly this path. bulkerlib/types and redshift_driver pass. Build and gofmt clean.

Risk

Highest blast radius of the three: this output feeds Snowflake COPY, Redshift COPY … csv gzip, ClickHouse url(), BigQuery's loader, and customers reading their own S3/GCS objects. The format is unchanged — only the size and CPU move — and the CI warehouse suites are the real check.

🤖 Generated with Claude Code

jitsu-code-review

Reviewed the gzip implementation substitutions in the SQL batch-writing and marshaller paths. No actionable bugs, security issues, or user-visible regressions found.

jitsu-code-review

Reviewed the warehouse batch gzip writer migration and the marshaller compression-level adjustment.

No actionable findings. The updated writers remain standard-gzip compatible, and the affected bulkerlib/types tests pass.

jitsu-code-review

Reviewed the gzip writer replacements and the compression-level adjustment in the warehouse batch paths. Verified the dependency is already present in the affected module and checked the focused marshaller tests. No actionable correctness, security, or user-visible regression found.

jitsu-code-review

Reviewed the gzip writer migration across the transactional batch path and JSON/CSV marshallers, including the new compatibility and compression-level coverage. No actionable bugs, security issues, or user-visible regressions found.

jitsu-code-review

Reviewed the gzip writer substitutions and marshaller-level compatibility coverage. I found no actionable correctness, security, or user-visible regression issues in this range.

`Bulker Test (app)` fails on every branch that actually runs it, and **`newjitsu` is currently red** for the same reason. ``` panic: create container: Error response from daemon: pull access denied for minio/minio, repository does not exist or may require 'docker login' ``` ## Cause `docker.io/minio/minio` is **no longer publicly pullable**. An anonymous pull token from `auth.docker.io` is rejected with `UNAUTHORIZED`, and Docker Hub's API has no public record of the repository. Docker's *"repository does not exist or may require 'docker login'"* is its generic message for an unresolvable reference — so this is **not** rate limiting, and adding registry credentials would not have fixed it. Reproduced locally with the identical error: ``` $ docker pull minio/minio:latest Error response from daemon: pull access denied for minio/minio, repository does not exist or may require 'docker login' ``` ## Fix MinIO also publish to **quay.io**, which is public. One line in `minio_container.go`. Pinned to `RELEASE.2025-09-07T16-13-09Z` rather than a floating tag. That tag is **multi-arch** (amd64 / arm64 / ppc64le) and its manifest list is currently **byte-identical** to quay's `:latest`, so pinning costs nothing today and keeps the tests runnable on Apple Silicon. Both verified by pulling: ``` $ docker pull quay.io/minio/minio:RELEASE.2025-09-07T16-13-09Z # arm64, native Status: Downloaded newer image $ docker pull --platform linux/amd64 quay.io/minio/minio:RELEASE.2025-09-07T16-13-09Z Status: Downloaded newer image ``` There is precedent for quay in the tree — `docker/docker-compose.yml:288` pulls Keycloak from it. ## Why this went unnoticed for six days The unpinned `:latest` meant the break needed no commit in this repo. And the bulker shard **skips whenever a PR does not touch `bulker/`**, reporting `pass` in ~11s — a real run takes 4-5 minutes. Recent merges (#1502, #1507, #1509, #1510) all show `Bulker Test (app): pass` at 11s; none of them touched `bulker/`. The last real run was #1503 on **9 September**. The JITSU-163 PRs were the first to exercise the suite since, which is why they surfaced it — and merging #1503 turned `newjitsu` red. Worth considering separately: a scheduled run of the full suite on `newjitsu` would have caught this within a day instead of six. ## Verification CI on this PR is the real test — it runs the exact job that is failing. Locally the image now pulls natively on both architectures, and the previous reference reproduces the CI failure exactly. Unblocks #1504, #1506, #1508 and the default branch. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

jitsu-code-review

Reviewed the gzip writer migration across the batch marshallers and transactional SQL conversion path, including compatibility coverage. No actionable bugs, security issues, or correctness regressions found.

jitsu-code-review

Reviewed the gzip implementation swap across the batch marshallers and transactional SQL conversion path, including output compatibility coverage and configured compression levels.

No actionable findings.

Third of the klauspost swap in JITSU-163, covering the warehouse batch path: the JSON, JSONArray and CSV marshallers, and the transactional SQL stream. The ticket names "all five warehouse adapters", but those files contain no gzip calls — GetBatchFileCompression() only returns a declared value, and redshift's `gzip` is the COPY ... csv gzip SQL keyword. The compression happens where the marshaller is constructed, so it resolves to these two. Includes the compression level change, which cannot be separated from the swap: klauspost at level 4 emits 9.5% larger output than the stdlib level 4 it replaces. Level 6 is the equivalent — 4.4% smaller and 25% cheaper — so the library and the level have to move together or the intermediate state is a regression on file size. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Asserts the two properties the klauspost swap depends on and that no existing test covers: - output from every compressing marshaller is readable by stdlib compress/gzip, which is what the warehouse COPY commands and customers reading their own S3/GCS objects rely on - the configured gzip level is actually applied, compared against a literal rather than the gzipLevel constant so the test cannot pass at the wrong level Also the only coverage of JSONArrayMarshaller's gzip branch: its sole consumer is the webhook destination, which returns FileCompressionNONE, so no configuration can reach it. JITSU-163 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

jitsu-code-review

Reviewed the gzip writer migration in the Bulker marshallers and transactional SQL batch conversion path, including the new stdlib-compatibility coverage. No actionable correctness, security, or user-visible regression findings.

@absorbb

Adds `FileCompressionZSTD` end to end for S3/GCS destinations and internal batch paths. **Merging this changes nothing at runtime** — every new zstd write is behind config defaulting to gzip, and the `jitsu-cloud-billing` change that flips the event-archive connection lands separately, once every reader is deployed. Stacked on #1506. That must merge first. **Updated after review:** the job-payload codec is removed — `admin` compresses job config with gzip as before and `reprocessing-worker` assumes gzip, per @absorbb. `admin` is now byte-identical to its pre-change state apart from the `failover_reprocessor` filters. Re-verified end to end afterwards on both backends. ## The framing that shaped the diff The ticket is organised by *where compression happens*. The risk is in *where filenames are built and matched*, and that list is longer. Every reader here picks its decompressor from a file name. So a missed compression branch fails loudly — no zstd gets written. A missed **naming** branch fails silently: zstd bytes under a gzip-shaped name are misread, skipped by discovery, or never cleaned up. Suffixes therefore live in one place — `jitsubase/compression`, the module every reader and writer already imports. ## Sites the ticket doesn't list Found by grepping the invariant rather than following the ticket's list. Ildar already asked for the `failover_reprocessor` filters in this PR; these are additional: - **`implementations/file.go` `AddFileExtension`** — the `switch` only handled GZIP, so a zstd object would be named `.ndjson` with zstd bytes inside. Live on the S3/GCS write path via `file_storage/abstract.go`. The worst of these. - **`s3.go` and `google_cloud_storage.go`** set `Content-Type`; zstd would have fallen through to `application/x-ndjson`. The ticket names these files but frames them only as reading `config.Compression`. - **`kafkabase/failover_logger.go`** globs `*.ndjson` / `*.ndjson.gz` when cleaning up, so `.zst` files would never be deleted — local disk grows unbounded. Different and worse than being skipped. - `FileExtension()` in the three marshallers, and the hardcoded `".gz"` on failover rotate. - `blockStorageSettings.compression` in the console schema — zstd wasn't selectable at all. Default stays gzip, per the ticket. ## Departures from the ticket **1. Encoder pools, not one encoder per destination.** A `Destination` is shared across one topic consumer per table × mode, so several batches run against it concurrently, and zstd's streaming writer isn't concurrency-safe. Not a preference — demonstrated. `TestZstdMarshallersAreSafeUnderConcurrency` runs 16 goroutines through the pool and passes under `-race`. Replacing the pool with one shared encoder, which is what the ticket says literally, produces `WARNING: DATA RACE` six times. **2. A shared suffix package.** Not in the ticket. Adds no dependency edges — all four modules already import `jitsubase`. Holds only strings and matching; codec wiring stays with the callers. **3. A small refactor.** The decompressor selection was inline in a large function in `reprocessing-worker`, which is why it had no coverage. Now `decompressorFor(path, r)`. **4. `file_storage/abstract.go` deliberately keeps stdlib `compress/gzip`** at its default level. It was never part of the klauspost swap, and routing it through the shared helper would change existing gzip output as a side effect of adding zstd. **This asymmetry is intentional and easy to "tidy" into a regression.** ## Verified in helm dev Binary provenance checked first — the running bulker carries this branch's symbols. | Destination | Object | Content-Type | |---|---|---| | S3 zstd | `zstd-ndjson/events_….ndjson.zst` | `application/zstd` | | S3 gzip (ndjson + csv) | `events_….ndjson.gz` / `.csv.gz` | — | | GCS zstd | `zstd-ndjson/events_….ndjson.zst` | `application/zstd` | | GCS gzip | `gzip-ndjson/events_….ndjson.gz` | `application/gzip` | ~12k events through `POST /api/s/s2s/batch`. The S3 zstd object is read by the `zstd` CLI and Python's stdlib; `file` reports Zstandard; 4000 valid JSON records. **The result that matters most:** on both backends the gzip and zstd objects decompress to **the same 4000 events** — equivalent record for record once field order is normalised, and the same 1,934,847 bytes in length. Evidence the gzip path is untouched. They are deliberately *not* compared byte for byte: `Object` is built from a Go map, so field order is randomised between runs and byte-equality would fail on correct code. An earlier version of this section claimed byte-identity, which was inferred from equal length rather than measured. GCS was tested against `fsouza/fake-gcs-server` via `STORAGE_EMULATOR_HOST`, not real GCS. ## Backward compatibility The reader and filter changes are **not** config-gated — they take effect on deploy — so each was checked as equivalent rather than assumed safe: - the discovery matcher is a **strict superset**: only `.ndjson.zst` newly matches, checked against the old logic including pathological double extensions - the cleanup globs add a pattern that matches nothing today - `decompressorFor` is identical for `.gz` and uncompressed - `CompressionExtension(GZIP)` is still `.gz`; `CompressionContentType(GZIP)` is still `application/gzip` ## Replay `TestReprocessIsIdenticalAcrossCodecs` drives the **real** reprocess function over real files, once per codec, requiring identical events, counts and `connection_ids`. Mutation-checked. With the zstd case removed: ``` TotalLines = 1, want 4 - the file was probably not decompressed ...ndjson.zst replayed [], want [e1 e2 e3 e4] ``` Zero events replayed, one parse error as the only signal — the exact failure the prerequisite exists to prevent. ## Memory The ticket flags +18–21 MB RSS per concurrent encoder; pooling was added on top, so it needed numbers. ~20.6 MB per concurrent encoder, flat from 1 to 64. **The pool does not retain them** — heap returns to baseline after two GCs (1154 MB → 660 MB after one GC, 0.4 MB after two). So the cost is peak, not a leak: **peak RSS scales linearly with concurrent zstd batches** — 64 at once is over 1 GB. `zstd.WithWindowSize` is the lever if it bites. ## What isn't covered - **Real S3/GCS** — minio and an emulator stand in. - **Streaming reads.** The replay test reads local files; production streams from object storage. Same `decompressorFor` either way. - `mixpanel.go` needs a real token. **Recommendation:** merge freely — it's a runtime no-op. Before flipping the archive to zstd, read one real `.zst` back through `reprocessing-worker` in the real environment. The config gate exists precisely so that can happen before any producer emits zstd. ## Notes - klauspost stays at **v1.19.1**, not the benchmarked v1.19.2 — proven byte-identical at levels 4 and 6 by SHA-256, so the ticket's numbers transfer. - One new env var: `FAILOVER_LOGGER_COMPRESSION_CODEC`, defaulting to `gzip`. - `operator.go` stays on gzip per the ticket. Worth noting it builds `.json.gz` keys in **three** places (977, 1108, 1220), not one, if that's ever revisited. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

`Bulker Test (app)` fails on every branch that actually runs it: panic: create container: Error response from daemon: pull access denied for minio/minio, repository does not exist or may require 'docker login' `docker.io/minio/minio` is no longer publicly pullable — an anonymous pull token is rejected with UNAUTHORIZED and Docker Hub has no public record of the repo. Docker's "repository does not exist or may require 'docker login'" is its generic message for an unresolvable reference, so this is not rate limiting and adding registry credentials would not have fixed it. Reproduced locally with the same error. MinIO also publish to quay.io, which is public. Pinned to RELEASE.2025-09-07T16-13-09Z rather than a floating tag: it is multi-arch (amd64/arm64/ppc64le) and its manifest list is currently byte-identical to quay's :latest, so pinning costs nothing today and keeps the tests runnable on Apple Silicon. The unpinned `:latest` is why this broke with no commit in the repo and stayed hidden for six days: the bulker shard skips whenever a PR does not touch `bulker/`, reporting "pass" in 11s, and nothing between 9 and 15 Sep touched it. The JITSU-163 PRs were the first to run the suite again, and merging jitsucom#1503 turned newjitsu red. Unblocks jitsucom#1504, jitsucom#1506, jitsucom#1508 and the default branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

jitsu-code-review Bot approved these changes Sep 9, 2026

View reviewed changes

sahiltyagi-jitsu force-pushed the feat/jitsu-163-klauspost-gzip-batch-writers branch from dd57084 to 319c03e Compare September 9, 2026 13:02

sahiltyagi-jitsu changed the title ~~JITSU-163: klauspost/gzip on the warehouse batch path~~ JITSU-163: klauspost/gzip on the warehouse batch path, with the level change Sep 9, 2026

jitsu-code-review Bot approved these changes Sep 9, 2026

View reviewed changes

jitsu-code-review Bot previously approved these changes Sep 9, 2026

View reviewed changes

sahiltyagi-jitsu added this pull request to stack #1505 September 9, 2026 15:19

sahiltyagi-jitsu dismissed jitsu-code-review[bot]’s stale review via d5ce61b September 10, 2026 12:12

jitsu-code-review Bot previously approved these changes Sep 10, 2026

View reviewed changes

absorbb previously approved these changes Sep 10, 2026

View reviewed changes

sahiltyagi-jitsu mentioned this pull request Sep 10, 2026

JITSU-163: zstd for S3/GCS destinations and internal batch paths #1508

Merged

sahiltyagi-jitsu dismissed stale reviews from absorbb and jitsu-code-review[bot] via 8bf8683 September 15, 2026 07:11

sahiltyagi-jitsu force-pushed the feat/jitsu-163-klauspost-gzip-batch-writers branch from d5ce61b to 8bf8683 Compare September 15, 2026 07:11

jitsu-code-review Bot previously approved these changes Sep 15, 2026

View reviewed changes

sahiltyagi-jitsu mentioned this pull request Sep 15, 2026

fix(bulker): pull the minio test container from quay, pinned #1520

Merged

sahiltyagi-jitsu dismissed jitsu-code-review[bot]’s stale review via 8d1ebec September 15, 2026 08:54

sahiltyagi-jitsu force-pushed the feat/jitsu-163-klauspost-gzip-batch-writers branch from 8bf8683 to 8d1ebec Compare September 15, 2026 08:54

jitsu-code-review Bot approved these changes Sep 15, 2026

View reviewed changes

Base automatically changed from feat/jitsu-163-klauspost-gzip-writers to newjitsu September 15, 2026 09:04

sahiltyagi-jitsu and others added 2 commits September 15, 2026 14:34

sahiltyagi-jitsu force-pushed the feat/jitsu-163-klauspost-gzip-batch-writers branch from 8d1ebec to 8c790a3 Compare September 15, 2026 09:04

jitsu-code-review Bot approved these changes Sep 15, 2026

View reviewed changes

sahiltyagi-jitsu merged commit 8f5d23c into newjitsu Sep 15, 2026
12 checks passed

sahiltyagi-jitsu deleted the feat/jitsu-163-klauspost-gzip-batch-writers branch September 15, 2026 09:12

Navigation Menu

Uh oh!

JITSU-163: klauspost/gzip on the warehouse batch path, with the level change - #1506

JITSU-163: klauspost/gzip on the warehouse batch path, with the level change#1506
sahiltyagi-jitsu merged 2 commits into
newjitsufrom
feat/jitsu-163-klauspost-gzip-batch-writers

sahiltyagi-jitsu commented Sep 9, 2026 •

edited

Loading

Uh oh!

jitsu-code-review Bot left a comment

Uh oh!

jitsu-code-review Bot left a comment

Uh oh!

jitsu-code-review Bot left a comment

Uh oh!

jitsu-code-review Bot left a comment

Uh oh!

jitsu-code-review Bot left a comment

Uh oh!

jitsu-code-review Bot left a comment

Uh oh!

jitsu-code-review Bot left a comment

Uh oh!

jitsu-code-review Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

sahiltyagi-jitsu commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The level change is part of this, deliberately

Which files, and why these

Verification

Risk

Uh oh!

jitsu-code-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

jitsu-code-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

jitsu-code-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

jitsu-code-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

jitsu-code-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

jitsu-code-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

jitsu-code-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

jitsu-code-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sahiltyagi-jitsu commented Sep 9, 2026 •

edited

Loading