fix(pulsar): adjust event finalization/acking#71
Draft
Conversation
Three changes here: 1. Always create the OrderedFinalizer and `ack_stream` Previously this was only done when acks were enabled (either globally, through the sink configuration, or the legacy source option). 2. Always route events through the finalizer, attaching BatchNotifier only when acknowledgements are configured Instead of preemptively acking messages to the broker after parsing when sink acks are **not** enabled, we instead use the `Drop` of the finalizer to signal when the event is processed and should be acked. This forces the broker acknowledgement through the `ack_stream` in the same way regardless of the sink configuration, which provides natural throttling of the consumer. This also matches the kafka behavior where they appear to have learned the same lesson. 3. Add biased; to the `tokio::select!` loop to prioritize ack processing over message consumption. Again, this matches kafka behavior: `src/sources/kafka.rs#L640-L645` Ref: LOG-23596
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.
Three changes here:
ack_streamPreviously this was only done when acks were enabled (either globally, through the sink configuration, or the legacy source option).
Instead of preemptively acking messages to the broker after parsing when sink acks are not enabled, we instead use the
Dropof the finalizer to signal when the event is processed and should be acked. This forces the broker acknowledgement through theack_streamin the same way regardless of the sink configuration, which provides natural throttling of the consumer. This also matches the kafka behavior where they appear to have learned the same lesson.tokio::select!loop to prioritize ack processing over message consumption.Again, this matches kafka behavior:
src/sources/kafka.rs#L640-L645Ref: LOG-23596