You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(ingest): exact WAL backlog and org queue counters, no metering for refused batches - #2440
Stacked on #2439. Part of the audit stack that starts at #2434.
The Rust in this PR was written without compiling it locally. The Ingest Rust tests job on this PR built it and its tests pass. The Go changes were built and tested locally.
Bugs fixed
The WAL backlog counter drifted up forever. A sync added the whole batch to the committed count, including frames whose caller had gone (request timeout, client disconnect). Their delivery was skipped, so they never reached the exported count. The backlog then never returned to zero: every shutdown waited the full drain window, and scale-in protection could stay on. A skipped delivery now takes its bytes back out.
A failed cursor write leaked org queue bytes. When mark_exported failed, the frames left memory but the org's queue counter kept their bytes until restart, throttling the org on a queue that no longer existed. The bytes are now released before the error is returned.
A batch the collector refused was still metered. In forward mode a relayed 4xx was returned as success and usage was recorded. Metering and key use are now skipped when the relay was refused and nothing was stored, on the HTTP and the gRPC path. In dual mode the batch is accepted into the WAL before it is forwarded, so a refusal there is still metered.
A corrupt frame header could size a 4 GiB allocation before the checksum was read. The payload is now read incrementally against the declared length.
INGEST_QUEUE_CHANNEL_CAPACITY=0 panicked at boot. It is rejected in config validation.
Go collector exporter encoded differently from the gateway. An unknown severity became "0" instead of "", and a timestamp above the int64 range wrapped instead of mapping to the epoch.
Removed
INGEST_TINYBIRD_CONCURRENCY_PER_SHARD: parsed, validated and forwarded, but nothing read it. There is one export worker per lane.
accept_rows: no callers.
Stale "Fargate ephemeral storage" comments and a Rust version in Dockerfile.prebuilt are corrected.
Verification
Go: go vet ./... and go test ./... pass. The new cases fail against the old encoding.go.
Rust: the Ingest Rust tests and Rust code scanning jobs pass. New tests cover the backlog counter, the cursor-write failure, the oversized frame header, the zero channel capacity, the refused relay over HTTP and gRPC, and the dual-mode rule.
Left for a decision
A failed export batch still leaves its bytes in the committed count, the same shape as the first fix. Fixing it means deciding whether to retry in process.
gRPC still answers OK when the collector refuses a batch. Only the metering changed.
After this merges, INGEST_TINYBIRD_CONCURRENCY_PER_SHARD can be dropped from the prd and dev deploy configs; the gateway ignores it.
An undrained backlog is exported twice when the host disk survives a restart.
Replay loads a whole lane into memory, and the shutdown flush re-uploads every sealed segment in sequence.
…ing refused batches
- WAL backlog no longer drifts up. A sync counted every frame in its batch as
committed, including frames whose caller had been dropped (request timeout,
client disconnect). Those are never delivered to export, so they were never
marked exported and the backlog never returned to 0: every shutdown waited
the full drain window and scale-in protection could stick. A skipped
delivery now takes its bytes back out of the lane's committed count.
- A failed cursor write no longer leaks org queue bytes. The batch leaves
memory either way, so the org's bytes are released before the error is
returned. The comment that claimed the batch "will be replayed" is corrected.
- A collector 4xx relayed in forward mode is no longer metered or billed, on
both the HTTP and gRPC paths (it already did not count as key use on HTTP).
- read_wal_frame no longer allocates the header's payload length (up to 4 GiB)
before the checksum: the buffer grows with the bytes actually read, and a
short read is a corrupt frame.
- INGEST_QUEUE_CHANNEL_CAPACITY=0 is refused at validation instead of
panicking in mpsc::channel(0) at boot.
- Remove INGEST_TINYBIRD_CONCURRENCY_PER_SHARD: it was parsed, validated and
forwarded but nothing read it (there is one export worker per lane).
- Remove accept_rows, which had no callers.
- Comments: the WAL lives on the EC2 instance store, not Fargate ephemeral
storage, and the prebuilt image compiles in rust:1.94-bookworm.
…ps like the gateway
- An unknown severity number is now the empty string, not its decimal form,
matching the gateway's and Maple Local's encoders.
- A timestamp above the int64 maximum is written as the epoch. It used to wrap
negative, and a value outside DateTime64(9) fails the whole insert.
- Drop the leftover unused-import guard in exporter_traces.go.
- README: org_id is stamped on every record; a resource attribute overrides it
only when org_id_from_resource_attribute is set, as config.go documents.
No actionable comments were generated in the recent review.
Walkthrough
The ingest service changes batch accounting, WAL handling, and configuration. The OTel exporter changes timestamp and severity encoding, updates its configuration example, and removes unused trace-exporter code.
Changes: Ingest pipeline and WAL
Layer / File(s)
Summary
Batch storage and metering apps/ingest/src/main.rs
HTTP and gRPC paths use batch storage classification when recording source use or metered bytes. Tests cover refused forwards and native-pipeline acceptance.
WAL reads, delivery, and queue accounting apps/ingest/src/telemetry.rs
WAL reads cap initial allocation and reject truncated payloads. Caller abandonment adjusts backlog accounting, and export workers release queue-byte reservations even when cursor marking fails. Tests cover these behaviors.
The per-shard export-concurrency setting and accept_rows method are removed. Validation rejects zero queue-channel capacity. Build and task settings, benchmark configuration, and WAL storage documentation also change.
OTel Maple exporter
Layer / File(s)
Summary
Timestamp and severity encoding packages/otel-collector-maple-exporter/internal/encoding.go, packages/otel-collector-maple-exporter/internal/encoding_test.go
Timestamp values above MaxInt64 now format as the epoch. Unknown severity numbers now encode as an empty string. Tests cover boundary timestamps and unknown severities.
Exporter example and trace cleanup packages/otel-collector-maple-exporter/README.md, packages/otel-collector-maple-exporter/exporter_traces.go
The README describes when org_id is applied and when a resource attribute overrides it. The trace exporter removes an unused import and its placeholder reference.
No concrete issue identified here blocks merging after normal checks. The Rust build and tests still need to run.
Pre-merge checks: 4 · 1
Failed checks: 1 inconclusive
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly summarizes the main ingest changes: correcting WAL backlog and organization queue counters, and skipping metering for refused batches.
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
Explanation
Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 12 functions across 5 files. (1 skipped: 1 too large.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 77f1d06f-a7dc-42cc-b28a-4e0925cb4b66
Commits
Reviewing files that changed from the base of the PR and between e36f5b4 and 39b9477.
Files selected for processing: 2
apps/ingest/src/main.rs
apps/ingest/src/telemetry.rs
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.
…tored the batch
In dual mode the batch is accepted into the WAL before it is forwarded, so a
collector refusal does not mean nothing was stored. Only a refusal that left
nothing behind skips metering and key use, on the HTTP and the gRPC path.
F1 (dual-mode rows unmetered) was fixed in the commit after the one reviewed and is resolved in its thread: metering and key use are skipped only when the relay was refused and the native pipeline did not take the batch.
F2 (no test for the gRPC metering skip) is addressed in the latest push: a_batch_the_collector_refuses_does_not_count_as_key_use now also calls accept_grpc_decoded with the same source key against the refusing collector, and asserts it answers OK and records no key use.
Both are uncompiled until the Ingest Rust tests job runs on this head.
🟢 Confidence 10/10 · safe to merge. Backlog and org-byte arithmetic verified by reading the lane sync/delivery ordering; the Rust test job has not finished at this head. no findings · tests covered · risk high
Fixes five WAL/metering defects in the ingest gateway plus two encoder mismatches in the Go collector exporter, and drops the unused INGEST_TINYBIRD_CONCURRENCY_PER_SHARD. The counter and billing changes hold up; safe to merge.
while_caller_waits subtracts a skipped frame's bytes from committed_bytes, so backlog returns to zero
export_and_mark releases org queue bytes before marked?, covering a failed cursor write
batch_was_stored skips metering and key use when a relay was refused and nothing was stored
read_wal_frame grows the payload buffer instead of allocating the declared length up front
Manual · Drop INGEST_TINYBIRD_CONCURRENCY_PER_SHARD from the prd and dev deploy configs; nothing reads it at this head
4434067 · What was checked, coverage and telemetry details are on the Maple / review check. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.
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
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.
![maple-review-bot[bot]](https://avatars.githubusercontent.com/in/4108169?s=60&v=4){kind=small}
Stacked on #2439. Part of the audit stack that starts at #2434.
The Rust in this PR was written without compiling it locally. The
Ingest Rust testsjob on this PR built it and its tests pass. The Go changes were built and tested locally.Bugs fixed
mark_exportedfailed, the frames left memory but the org's queue counter kept their bytes until restart, throttling the org on a queue that no longer existed. The bytes are now released before the error is returned.INGEST_QUEUE_CHANNEL_CAPACITY=0panicked at boot. It is rejected in config validation."0"instead of"", and a timestamp above the int64 range wrapped instead of mapping to the epoch.Removed
INGEST_TINYBIRD_CONCURRENCY_PER_SHARD: parsed, validated and forwarded, but nothing read it. There is one export worker per lane.accept_rows: no callers.Stale "Fargate ephemeral storage" comments and a Rust version in
Dockerfile.prebuiltare corrected.Verification
go vet ./...andgo test ./...pass. The new cases fail against the oldencoding.go.Ingest Rust testsand Rust code scanning jobs pass. New tests cover the backlog counter, the cursor-write failure, the oversized frame header, the zero channel capacity, the refused relay over HTTP and gRPC, and the dual-mode rule.Left for a decision
INGEST_TINYBIRD_CONCURRENCY_PER_SHARDcan be dropped from the prd and dev deploy configs; the gateway ignores it.