Ignore duplicate client requests #379
Merged
Conversation
This file contains 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
Comment on lines
86
to
89
| else | ||
| { | ||
| return; // this is a duplicate. Ignore it. | ||
| } |
There was a problem hiding this comment.
nit: given that this is rare - should we log it? Might be helpful to know if EH is acting up and duplicating events a lot.
davidmrdavid
approved these changes
Apr 15, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
During stress testing we observed to our surprise that event hubs sometimes duplicates events internally. Specifically, it is possible for an event to be successfully sent by a partition client only once, but then this event appears twice in the delivery sequence to the partition, with two different sequence numbers. Specifically we saw the same event being delivered at positions 2115 and 2121. The events in between (positions 2116 to 2120) corresponded to events that were sent from a different source.
Thankfully the practical impact of this discovery is relatively minor since we are already deduplicating almost all events. The only events not being duplicated are client requests that are sent from a client to a partition.
Netherite was designed assuming this would not happen, so we detected this because an assertion violation triggered.
In this PR we modify the code so that if a duplicate is detected, we ignore it instead of creating an assertion violation error.
Note that this is not a reliable client request deduplication, because if the duplicate arrives much later than the original, the duplicate will be processed a second time. However I think this is an o.k. approximation since any clients that are sensitive to duplication problems should already use idempotent versions of the API calls (such as including deduplication status when creating an orchestration) which would not be affected by internal event duplication.