-
Notifications
You must be signed in to change notification settings - Fork 0
Add defenesive deletion tests #21
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
base: main
Are you sure you want to change the base?
Conversation
marcus-pousette
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left a partial review
| assert!(crdt_a.children(parent).unwrap().is_empty()); | ||
|
|
||
| // Client B doesn't know about deletion yet, inserts child under parent | ||
| // We need to ensure Client B's clock is advanced so the insert has higher lamport |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if the higher lamport criteria is needed. From what I understood from the "defensive delete" is that it does not depend on the clock, but only the reason that someone deleted something while not being synced with another peer, that would make the "another peer" to make a pingpong-back-revival move once this peer observes this delete move
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Yes, it make sense. Client could delete node even after insertion, but the main criteria should be that client is not aware of changes made by other client.
| assert!(!crdt_b.is_tombstoned(parent)); // Parent not tombstoned in B's view | ||
|
|
||
| // Now synchronize: Client B receives the delete | ||
| crdt_b.apply_remote(delete_op.clone()).unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder actually here in a defensivie delted behaviour this should be
// applying the remote deletion node will first apply deletion according to the kleppman spec, but this would also construct revival move
// pseudo code
let revival_ops = crdt_b.apply_remote(delete_op.clone(), { deleteBehaviour: 'defensive' }).unwrap();
crdt_a.apply_remote(insert_child).unwrap(); // no revival yet
crdt_a.apply_remote(revival_ops).unwrap(); // here we realize that client b reallise want that the child should exist!
No description provided.