-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat(operation): Introducing operation entity and thread safe operation queue. #16
base: main
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## main #16 +/- ##
==========================================
- Coverage 76.16% 73.05% -3.11%
==========================================
Files 6 6
Lines 193 193
==========================================
- Hits 147 141 -6
- Misses 39 45 +6
Partials 7 7
Continue to review full report at Codecov.
|
|
||
// Operation - individual operation entity | ||
Operation struct { | ||
Id *lamport.Clock |
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.
why are we using pointers here?
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.
To prevent it from unnecessary being copied and to have the atomicity while checking and updating the counter value.
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.
at operation level the clock is fixed as is used as an id so no updates.
Suggestion:
don't use pointers here
rest looks good
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.
It hardly makes any difference. If you insist - value then.
if atomic.LoadUint64(&obj.counter) == atomic.LoadUint64(&clock.counter) { | ||
return clock.hostId < obj.hostId | ||
} else { | ||
return atomic.LoadUint64(&clock.counter) < atomic.LoadUint64(&obj.counter) |
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.
revert
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.
(counter, hostId) defines total ordering of operations across all replicas
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.
Just think logically and tell me, how does it differ when the two operations happing at two different hosts have the same Lamport counter. The ordering of the host id should matter? No. Either the host id is auto-generated at init or randomly given during bootstrapping. So why should we take that into consideration inside the programming logic? Just keep it for the identification purpose - like where the counter was set.
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.
To resolve concurrent operations, For LWW to work we will need someone to win.
If a replica receives an operation with a counter value c
that is greater than the locally stored counter value, the local
counter is increased to the value of the incoming counter.
This ensures that if operation o1 causally happened before
o2 (that is, the replica that generated o2 had received and
processed o1 before o2 was generated), then o2 must have
a greater counter value than o1. Only concurrent operations
can have equal counter values.
We can thus define a total ordering < for Lamport
timestamps:
(c1, p1) < (c2, p2) iff (c1 < c2) ∨(c1 = c2 ∧p1 < p2).
If one operation happened before another, this ordering is
consistent with causality (the earlier operation has a lower
timestamp). If two operations are concurrent, their order
according to < is arbitrary but deterministic. This ordering
property is important for our definition
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.
Yea, I understand. We need to think about this constructively...instead of in such a destructive way like the last writer wins. As CRDT is all about resolving conflict optimistically : )
The topic for discussion on next meet.
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.
what if both have random ids which are coincidentally equal. We'll end up introducing randomness into the system
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.
each replica must have a unique id
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.
you can't ensure must. Instead we could focus here
Another frequently-used approach to conflict resolution
is last writer wins (LWW), which arbitrarily chooses one
among several concurrent writes as “winner” and discards
the others. LWW is used in Apache Cassandra, for example.
It does not meet our requirements, since we want no user
input to be lost due to concurrent modifications.
Features
TODO: tests
issue: #10