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

fix(storage-manager): incorrectly revive key after deletion #1689

Merged
merged 1 commit into from
Jan 7, 2025

Conversation

J-Loudet
Copy link
Contributor

@J-Loudet J-Loudet commented Jan 6, 2025

This commit fixes a bug where a key would be incorrectly "revived" in a scenario where a deletion is followed by a Wildcard Put.

Assuming we have two Replicas, R1 and R2, the problematic scenario was:

  1. Replicas R1 and R2 are disconnected.
  2. z_put "a" = 1 @t1 on Replica R1.
  3. z_delete "a" @t2 on Replica R2.
  4. z_put "**" = 42 @t3 on Replica R2.
  5. Connect Replicas R1 and R2.

After aligning, before this fix, Replicas R1 and R2 would have "a" = 42 @t3 in their storage whereas there should have been no key associated to "a" because of the delete at @t2.

This commit fixes this bug.

  • plugins/zenoh-plugin-storage-manager/src/replication/core/aligner_reply.rs: instead of eagerly applying a Wildcard Update that should not be applied, do nothing. The comments were updated to provide more context.

This commit fixes a bug where a key would be incorrectly "revived" in a
scenario where a deletion is followed by a Wildcard Put.

Assuming we have two Replicas, R1 and R2, the problematic scenario was:

0. Replicas R1 and R2 are disconnected.
1. `z_put "a" = 1 @t1` on Replica R1.
2. `z_delete "a" @t2` on Replica R2.
3. `z_put "**" = 42 @t3` on Replica R2.
4. Connect Replicas R1 and R2.

After aligning, before this fix, Replicas R1 and R2 would have `"a" = 42
@t3` in their storage whereas there should have been no key associated
to `"a"` because of the delete at `@t2`.

This commit fixes this bug.

* plugins/zenoh-plugin-storage-manager/src/replication/core/aligner_reply.rs:
  instead of eagerly applying a Wildcard Update that should not be
  applied, do nothing. The comments were updated to provide more
  context.

Signed-off-by: Julien Loudet <[email protected]>
@J-Loudet J-Loudet requested a review from oteffahi January 6, 2025 13:35
Copy link

github-actions bot commented Jan 6, 2025

PR missing one of the required labels: {'breaking-change', 'new feature', 'internal', 'bug', 'documentation', 'dependencies', 'enhancement'}

@J-Loudet J-Loudet added the bug Something isn't working label Jan 6, 2025
Copy link
Contributor

@oteffahi oteffahi left a comment

Choose a reason for hiding this comment

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

Tested and validated internally.
LGTM.

@J-Loudet J-Loudet merged commit 8dd0582 into main Jan 7, 2025
26 of 27 checks passed
@J-Loudet J-Loudet deleted the fix/replication/revival branch January 7, 2025 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants