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

Delta validation #130

Merged
merged 8 commits into from
Mar 7, 2024
Merged

Delta validation #130

merged 8 commits into from
Mar 7, 2024

Conversation

fulmicoton
Copy link
Contributor

@fulmicoton fulmicoton commented Feb 26, 2024

Bugfix: validation of delta before applying them.

Due to the nature UDP, the existence of resets and the fact that we are
gossipping to several nodes at the same time, it is possible for our
obsolete deltas to arrive.

This PR adds some validation to detect if the delta is valid, and
whether it will bring us to a better state or not.

It also removes the nodes to reset information, which was actually
taking a large amount of the MTU on large clusters.
(For 20 nodes, around 1KB)

Reset is now just expressed by sending the delta with `from_version = 0`.

Closes #129

@fulmicoton fulmicoton force-pushed the issue/128-delta-validation branch 6 times, most recently from 786bf89 to 2d9549e Compare February 26, 2024 09:51
@fulmicoton fulmicoton changed the title Issue/128 delta validation Delta validation Feb 26, 2024
@fulmicoton fulmicoton force-pushed the issue/128-delta-validation branch 4 times, most recently from 4f64387 to 3cfc3e3 Compare February 27, 2024 03:07
@fulmicoton fulmicoton marked this pull request as ready for review February 27, 2024 03:08
@fulmicoton fulmicoton force-pushed the issue/128-delta-validation branch from 3cfc3e3 to bdcd8c6 Compare February 27, 2024 03:10
@fulmicoton fulmicoton requested a review from fmassot February 27, 2024 04:32
Due to the nature UDP, the existence of resets and the fact that we are
gossipping to several nodes at the same time, it is possible for our
obsolete deltas to arrive.

This PR adds some validation to detect if the delta is valid, and
whether it will bring us to a better state or not.

It also removes the nodes to reset information, which was actually
taking a large amount of the MTU on large clusters.
(For 20 nodes, around 1KB)

Reset is now just expressed by sending the delta with `from_version = 0`.

Closes #129

-

Removing hidden contract

We avoid computing tombstone's Instant upon deserialization.
It was hiding a very hidden contract forcing us to deserialize mutation
in the order of their version.

With this change, we defer the computation of the instant to the
call of the apply_delta method. All of the tombstone from a delta
get the exact same `Instant`.
@fulmicoton fulmicoton force-pushed the issue/128-delta-validation branch from 37c0ccf to c55c883 Compare March 4, 2024 08:01
chitchat/src/delta.rs Outdated Show resolved Hide resolved
chitchat/src/delta.rs Outdated Show resolved Hide resolved
chitchat/src/state.rs Show resolved Hide resolved
chitchat/src/state.rs Outdated Show resolved Hide resolved
chitchat/src/state.rs Outdated Show resolved Hide resolved
chitchat/src/state.rs Show resolved Hide resolved
chitchat/src/state.rs Show resolved Hide resolved
chitchat/src/state.rs Outdated Show resolved Hide resolved
chitchat/src/state.rs Outdated Show resolved Hide resolved
@fulmicoton fulmicoton force-pushed the issue/128-delta-validation branch from 321a2de to 094d986 Compare March 7, 2024 03:16
Co-authored-by: Raphaël Marinier <[email protected]>
@fulmicoton fulmicoton force-pushed the issue/128-delta-validation branch from 565d538 to 6c77259 Compare March 7, 2024 04:44
@fulmicoton fulmicoton force-pushed the issue/128-delta-validation branch 2 times, most recently from 9d376e6 to 4d2a9b3 Compare March 7, 2024 08:20
@fulmicoton fulmicoton force-pushed the issue/128-delta-validation branch from 4d2a9b3 to 65dfd8c Compare March 7, 2024 08:25
ALGORITHM.md Outdated Show resolved Hide resolved
ALGORITHM.md Show resolved Hide resolved
ALGORITHM.md Outdated Show resolved Hide resolved
chitchat/src/state.rs Show resolved Hide resolved
chitchat/src/state.rs Show resolved Hide resolved
ALGORITHM.md Outdated Show resolved Hide resolved
@fulmicoton fulmicoton force-pushed the issue/128-delta-validation branch from e6b51f4 to 5b64937 Compare March 7, 2024 12:35
@fulmicoton fulmicoton merged commit 78f8aff into main Mar 7, 2024
2 checks passed
@fulmicoton fulmicoton deleted the issue/128-delta-validation branch March 7, 2024 12:38
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.

Delta validation
2 participants