Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIXED] (2.11) Deterministic clustered dedupe #6415

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

MauriceVanVeen
Copy link
Member

@MauriceVanVeen MauriceVanVeen commented Jan 27, 2025

Issue description:

  • Publish a message with a Nats-Msg-Id.
  • Leader election through restarts/networking/etc.
  • Due to leader election the proposal queue is drained and the message is not stored/proposed.
  • Any retries of the original message will fail until the de-dupe window clears the original message, even though it was not stored in the stream.

This issue is due to staging a zero sequence in the de-dupe map. An easy solution seems to clear all zero sequence entries from the de-dupe map upon stepping down, but that has correctness issues if any of those messages did get proposed.

This PR removes the staging of a zero sequence and ensures all replicas can deterministically do de-duping themselves. And duplicate messages are only blocked at the cluster-level if we know what the sequence is.

Signed-off-by: Maurice van Veen github@mauricevanveen.com

Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
@MauriceVanVeen MauriceVanVeen requested a review from a team as a code owner January 27, 2025 17:02
Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

LGTM

@derekcollison derekcollison merged commit ddc3655 into main Jan 27, 2025
5 checks passed
@derekcollison derekcollison deleted the maurice/deterministic-clustered-dedupe branch January 27, 2025 19:24
MauriceVanVeen pushed a commit that referenced this pull request Jan 28, 2025
Issue description:
- Publish a message with a `Nats-Msg-Id`.
- Leader election through restarts/networking/etc.
- Due to leader election the proposal queue is drained and the message
is not stored/proposed.
- Any retries of the original message will fail until the de-dupe window
clears the original message, even though it was not stored in the
stream.

This issue is due to staging a zero sequence in the de-dupe map. An easy
solution seems to clear all zero sequence entries from the de-dupe map
upon stepping down, but that has correctness issues if any of those
messages did get proposed.

This PR removes the staging of a zero sequence and ensures all replicas
can deterministically do de-duping themselves. And duplicate messages
are only blocked at the cluster-level if we know what the sequence is.

Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
@MauriceVanVeen MauriceVanVeen changed the title [FIXED] Deterministic clustered dedupe [FIXED] (2.11) Deterministic clustered dedupe Jan 29, 2025
derekcollison added a commit that referenced this pull request Jan 29, 2025
Since the introduction of
#6415 we have a deterministic
way of de-duplication such that leader changes are no issue as well as
never returning a zero-sequence or duplicate conflict error.

However, during an upgrade this could result in stream desync. This was
due to a replica accepting anything the leader tells it to do, ignoring
if the message is a duplicate. So if the leader running the new version
would propose multiple duplicate messages (which is correct under the
new behavior), the old version would ingest all messages instead of only
the first.

This PR fixes that by having the leader only do these types of proposals
if all servers within the group support it. Otherwise it falls back to
the previous implementation.

Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
MauriceVanVeen added a commit that referenced this pull request Feb 10, 2025
This reverts commit ddc3655, reversing
changes made to 52df7e6.
MauriceVanVeen added a commit that referenced this pull request Feb 10, 2025
This reverts commit ddc3655, reversing
changes made to 52df7e6.
derekcollison added a commit that referenced this pull request Feb 10, 2025
Reverts #6415 and
#6426. If duplication would
be deferred to be done by a replica, and that replica was down for at
least 'dupe window+5s', and it would clean up the dedupe map. A message
that was meant to be duplicate could be passed as a genuine message,
resulting in stream desync.

Instead, the dedupe map should be cleared of any staged zero-sequences.
However, that was not possible before as a new leader would not always
be fully up-to-date when it starts responding to new write requests,
which could result in duplicate messages as well.

However, relying on the 'Leader Completeness Property' implemented here:
#6485, we can confidently
clear the dedupe map now of any staged zero-sequences (knowing they were
not proposed). Ensuring both there's no desync, and a failed proposal
for a message would not block subsequent messages with the same dedupe
ID.

Have left the commits as separate, to ease reviewing.

Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
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.

2 participants