Skip to content

fix: Partial acceptance of records in ProduceSync - #21781

Merged
grobinson-grafana merged 2 commits into
mainfrom
grobinson/enforce-limit-on-all-records-produce-sync
May 7, 2026
Merged

fix: Partial acceptance of records in ProduceSync#21781
grobinson-grafana merged 2 commits into
mainfrom
grobinson/enforce-limit-on-all-records-produce-sync

Conversation

@grobinson-grafana

@grobinson-grafana grobinson-grafana commented May 7, 2026

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

This commit fixes situations where some records in a batch can be accepted, but not others. For example, consider a limit of 1KB, and records of size 512B, 513B and 256B. In this case, records 1 and 3 will be accepted, and 2 will be rejected. In most cases, this results in retries, and duplication of data. This change means that the entire batch is rejected if any records exceed the limit.

Same fix in Mimir: grafana/mimir#15227

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • Title matches the required conventional commits format, see here
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

Note

Medium Risk
Changes producer backpressure behavior by failing an entire batch when the buffered-bytes limit would be exceeded, which could affect ingestion under load. Logic is localized and covered by a new unit test, keeping overall risk moderate.

Overview
Producer.ProduceSync now enforces the buffered-bytes limit atomically per batch: it sums record sizes up-front and, if the batch would exceed maxBufferedBytes, it fails all records with kgo.ErrMaxBuffered instead of partially accepting some.

Adds a test ensuring oversized batches are fully rejected and that the same records succeed when produced individually.

Reviewed by Cursor Bugbot for commit ac8701b. Bugbot is set up for automated code reviews on this repo. Configure here.

This commit fixes situations where some records in a batch can be
accepted, but not others. For example, consider a limit of 1KB,
and records of size 512B, 513B and 256B. In this case, records
1 and 3 will be accepted, and 2 will be rejected. In most cases,
this results in retries, and duplication of data. This change
means that the entire batch is rejected if any records exceed
the limit.
@grobinson-grafana
grobinson-grafana requested a review from a team as a code owner May 7, 2026 06:55

@ashwanthgoli ashwanthgoli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@grobinson-grafana
grobinson-grafana merged commit 5f0e699 into main May 7, 2026
84 checks passed
@grobinson-grafana
grobinson-grafana deleted the grobinson/enforce-limit-on-all-records-produce-sync branch May 7, 2026 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2 participants