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

Incorrect CRDT Merge direction #1066

Closed
jsimnz opened this issue Jan 31, 2023 · 1 comment · Fixed by #2016
Closed

Incorrect CRDT Merge direction #1066

jsimnz opened this issue Jan 31, 2023 · 1 comment · Fixed by #2016
Assignees
Labels
area/crdt Related to the (Merkle) CRDT system priority/high refactor This issue specific to or requires *notable* refactoring of existing codebases and components
Milestone

Comments

@jsimnz
Copy link
Member

jsimnz commented Jan 31, 2023

At the moment we merge CRDT delta state starting from the HEAD and working backwards through the DAG, "merging" along the way. This is technically wrong, as it merges state backward to how its created.

However, the database still works because the primary CRDT type we use is the LWWRegister. Registers technically only care about the "last" value that is written, so if I set "name" to "Alice" then to "Bob", in reality, the in between state of "Alice" doesn't really matter that much.

However, the discussions related to #1064 its clear that this will breakdown for other CRDT types. Specifically this will already break down for Composite CRDTs, where the current operations are only CREATE, and UPDATE effectively. But we want to introduce a new operation DELETE, which is order dependent in which direction we "merge".

This will be necessary for #101 as well, since they'll likely be order dependant types.

@jsimnz jsimnz added area/crdt Related to the (Merkle) CRDT system refactor This issue specific to or requires *notable* refactoring of existing codebases and components labels Jan 31, 2023
@fredcarle fredcarle added this to the DefraDB v0.6 milestone May 29, 2023
@fredcarle
Copy link
Collaborator

Ping John when working on this.

@fredcarle fredcarle modified the milestones: DefraDB v0.6, DefraDB v0.7 Jul 10, 2023
@jsimnz jsimnz modified the milestones: DefraDB v0.7, DefraDB v0.8 Sep 18, 2023
fredcarle added a commit that referenced this issue Nov 1, 2023
## Relevant issue(s)

Resolves #1066 

## Description

This PR change the CRDT merge direction. Originally, when an update
arrived over the P2P network, each block was added to the DAG store and
merged into the datastore in the order that they were received (newest
to oldest block).

With this update, we start my receiving all blocks up to the version
that we currently have (or until we reach the root) and then we apply
the merge from oldest to newest block. This will enable more CRDT types
to be added.
shahzadlone pushed a commit to shahzadlone/defradb that referenced this issue Feb 23, 2024
## Relevant issue(s)

Resolves sourcenetwork#1066 

## Description

This PR change the CRDT merge direction. Originally, when an update
arrived over the P2P network, each block was added to the DAG store and
merged into the datastore in the order that they were received (newest
to oldest block).

With this update, we start my receiving all blocks up to the version
that we currently have (or until we reach the root) and then we apply
the merge from oldest to newest block. This will enable more CRDT types
to be added.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/crdt Related to the (Merkle) CRDT system priority/high refactor This issue specific to or requires *notable* refactoring of existing codebases and components
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants