Skip to content

controlplane/telemetry: keep newest samples on partial buffer overcapacity - #4233

Open
ayushsingh82 wants to merge 1 commit into
malbeclabs:mainfrom
ayushsingh82:fix/submitter-keep-newest-on-overcapacity
Open

controlplane/telemetry: keep newest samples on partial buffer overcapacity#4233
ayushsingh82 wants to merge 1 commit into
malbeclabs:mainfrom
ayushsingh82:fix/submitter-keep-newest-on-overcapacity

Conversation

@ayushsingh82

Copy link
Copy Markdown
Contributor

Summary

Fixes #4149.

  • Tick()'s failed-retry path (controlplane/telemetry/internal/telemetry/submitter.go) replaced the binary "does the whole unwritten remainder fit, yes/no" check with room := capacity - bufLen.
  • When the unwritten backlog doesn't fully fit, keeps the newest room samples (the tail — batches submit oldest-first, so unwritten is already chronological) and drops only the oldest excess, instead of discarding the entire backlog.
  • Fixes an off-by-one where an exact-fit backlog (bufLen + len(unwritten) == capacity) was treated as over capacity and dropped in full; it now requeues in full.

Test coverage

Rewrote 4 pre-existing tests whose assertions baked in the old boundary bug, rather than just adding new ones:

  • failed_retries_drop_when_over_capacityfailed_retries_keep_newest_when_over_capacity: was asserting full drop at a boundary that should now partially keep.
  • no_backpressure_when_drop_on_overcapacitykept_samples_never_exceed_capacity_after_partial_drop: retargeted at the invariant that actually matters post-fix (buffer never exceeds capacity), since "producer never blocks" isn't true in general once partial-keep can fill a buffer to exactly capacity.
  • drops_failed_samples_when_requeue_would_meet_capacityrequeues_failed_samples_when_they_exactly_meet_capacity: exact-fit (bufLen + len(unwritten) == capacity) now requeues in full instead of dropping — that was the off-by-one in the old >= check.
  • logs_and_counts_dropped_samples_when_over_capacity: updated to a genuine 3-samples-into-capacity-2 case so it exercises partial drop and asserts the new keptSamples log field.

Test plan

  • go test ./internal/telemetry/... — 8/8 top-level tests pass, 0 failures (includes all 3 renamed subtests)
  • go vet ./internal/telemetry/...
  • gofmt -l clean
  • golangci-lint run ./internal/telemetry/... — 0 issues

…acity

Tick()'s failed-retry path used to drop the entire unwritten batch the
moment it didn't fully fit back into the buffer. Replace that binary
check with room := capacity - bufLen, and requeue only the newest
`room` samples (unwritten is already oldest-first), dropping just the
oldest excess instead of the whole backlog. Also fixes an off-by-one
where an exact-fit backlog (bufLen + len(unwritten) == capacity) was
treated as over capacity and dropped in full.

Updates the tests whose assertions baked in the old boundary bug.
@ayushsingh82
ayushsingh82 requested a review from a team August 26, 2026 05:54
@ayushsingh82

Copy link
Copy Markdown
Contributor Author

@elitegreg PR is up, ready for CI.

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.

device/telemetry: over-capacity submitter path discards the whole backlog, not the oldest slice

1 participant