Skip to content

fix(batch-imports): decode all members of concatenated gzip files - #90036

Open
pl wants to merge 3 commits into
masterfrom
pl/ingestion/import-multi-member-gzip
Open

fix(batch-imports): decode all members of concatenated gzip files#90036
pl wants to merge 3 commits into
masterfrom
pl/ingestion/import-multi-member-gzip

Conversation

@pl

@pl pl commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Problem

  • A managed migration from gzipped S3 files imports only the first event of each file when the file holds concatenated gzip members, and reports the job complete. No error, no warning.
  • PostHog's own S3 batch exports write exactly this shape: the JSONL pipeline compresses each record as its own gzip member and concatenates them (per-record compression).
  • GzDecoder stops at the first member's end, which is indistinguishable from the end of the file, so the extractor records a one-event part as complete.

Changes

  • Gzip imports now decode every member of a file, so batch-export-shaped files import completely. MultiGzDecoder replaces GzDecoder in both producers (s3_gzip files, and gzip inside Amplitude zip entries); single-member streams decode identically.
  • The producers now fill a full 64KB block across reads before sending it to the consumer. One decoder read never crosses a member boundary, so per-record members previously meant one allocation and one channel send per record.
  • Mechanical: comment updates in the extractor and staging pipeline.

Note

Two loud-failure behavior changes, both replacing silent truncation:

  • A file with non-gzip bytes after the last member now pauses the job with a decode error instead of ignoring those bytes. They may be data.
  • A non-final member whose content lacks a trailing newline fuses two records into one unparseable line and pauses the job. Batch exports terminate every record with a newline, so only hand-concatenated files hit this.

Warning

Rollout notes:

  • Temp-bucket staging attaches existing staged plaintext without re-decoding, so a multi-member job in flight across the deploy keeps its current part truncated. Such imports, and any that completed before this fix, need a re-run; they are identifiable by an event count of roughly one per source file.
  • Rollback hazard: an old-image pod resuming a mid-part multi-member job sees a shorter stream than the persisted offset and marks the part complete. Log signature: "resumed at offset past the stream end; marking complete".

How did you test this code?

  • test_plain_gzip_extractor_concatenated_members: three per-record members; fails under a single-member decoder, which stops after the first record with a clean EOF.
  • test_zip_gzip_json_extractor_concatenated_members_in_entry: the same regression on the zip path; the two decoder call sites are independent.
  • test_plain_gzip_extractor_trailing_garbage_errors: pins that trailing non-gzip bytes surface an error rather than a clean EOF.
  • The multi-member claim was verified by executing the export pipeline's dump_record_batch with gzip: 3 records produced 3 gzip members, and a single-member decode returned only the first record with 110 bytes unread.
  • Ran the crate's lib tests, clippy, and fmt locally. Not run: the minio/seaweedfs integration suites, which need the docker stack; CI runs them.

Automatic notifications

  • Publish to changelog?

Docs update

None in this repo.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

  • Authored with Claude Code. Skills invoked: /writing-tests, /writing-code-comments, /reviewing-before-pr, /writing-pr-descriptions.
  • Found while verifying export/import compatibility between S3 batch exports and managed migrations.
  • hogli review could not run on this machine (Greptile CLI install fails with npm EPERM; reported via devex feedback), so the local pass was the harness fallback review. The independent review is the PR bot's.
  • Fallback review dispositions. Fixed: per-record channel sends (coalesced), zip-path test gap, fixture-helper duplication, three comment issues. Documented instead of coded: trailing-garbage errors (pinned by test, message wording unchanged), non-final-member newline fusion (loud failure, batch exports unaffected), cross-deploy staged-plaintext and rollback hazards (rollout notes above), grew-case total_size revalidation (possible follow-up in job resume logic). Rejected: folding the multi-member case into the newline-normalization table, because the fixture shape differs (that table's builder is single-member).

@pl pl self-assigned this Aug 27, 2026
@trunk-io

trunk-io Bot commented Aug 27, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@github-actions

Copy link
Copy Markdown
Contributor

🤖 CI report

Trunk lane — non-backend lane

This PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes.

@trunk-io

trunk-io Bot commented Aug 27, 2026

Copy link
Copy Markdown

Static BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

@pl
pl marked this pull request as ready for review August 27, 2026 12:40
@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Reviews (1): Last reviewed commit: "chore(batch-imports): extend gzip extrac..." | Re-trigger Greptile

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.

1 participant