fix: sink partial-failure handling, determinism, and hot-path allocations#147
Merged
Conversation
Kinesis PutRecords, SNS PublishBatch, and DynamoDB BatchWrite all return HTTP 200 while individual records are rejected or left unprocessed, so a success was reported even when work was dropped. Inspect the response: PutRecords returns ErrPartialFailure on a non-zero FailedRecordCount, a new BatchWriteChecked returns ErrUnprocessedItems on UnprocessedItems, and PublishBatch reports failed entries. EventBridge now reports the authoritative FailedEntryCount so the count and its plural stay correct. Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
…batch errors Prometheus serialized labels in map-iteration order, breaking the documented stable output and golden assertions; sort the keys. The statsd Aggregator dropped the caller context before the registry transform, losing deadline and trace propagation; thread it through metricFor. The reservoir non-batch dispatch kept only the last error; join them so no per-payload failure is lost. Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
Manifold.Attach is now copy-on-write so Sink reads the outlet slice without copying it on every emit. The http Post builds its request body with bytes.NewReader instead of copying through a string. CloudWatch gains a clock-injectable PutLogEventAt so the timestamp is testable without the wall clock. The unread WithPollerClock option and its stored-but-unused clock are removed, and the PhaseTransform godoc now states it is reserved rather than emitted by the built-in Emitter. Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
Add the http build-request and PostJSON marshal-error paths, and exercise the sinktest conformance harness's Flusher and Shutdowner error and panic branches plus its nil-Outlet guard. Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What this change does
Remediates the sink review findings (correctness + perf + coverage; no feature work):
kinesis.PutRecordsnow inspectsFailedRecordCountand returns a sentinelErrPartialFailure;dynamoaddsBatchWriteChecked(sentinelErrUnprocessedItems);eventbridgecounts from the authoritativeFailedEntryCount;sns.PublishBatchchecksout.Failed; the core reservoir non-batch patherrors.Joins all per-payload errors instead of keeping only the last.context.Background().Manifoldoutlets are copy-on-write so emit no longer copies the slice;httpusesbytes.NewReader;cloudwatchgains a clock-injectable variant.WithPollerClockoption (wiring needed new machinery);PhaseTransformkept but documented as reserved (removing it would be breaking) and the post-Shutdownreservoir path is guarded.-race).Checklist
examples/sinkflow+e2ebuild/test/vet clean; core+statsd-raceclean