-
Notifications
You must be signed in to change notification settings - Fork 105
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
Event log #290
Conversation
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.
Codewise this looks generally good to me, with a couple small questions. But I have some broader design questions before I stamp.
Let me know if you'd prefer design discussion here or in your tech proposal.
Trying to think through this scenario.
- Say that DWN A and B both have record R. Initially they have the same most recent update of record R.
- Then DWN A receives a RecordsWrite(A) updating R to be R(A). Slightly after that, before any sync occurs, DWN B also receives a RecordsWrite(B) updating R to be R(B).
- Is it deterministic which version of R wins? If not, is that acceptable?
If DWN A initiates a pull from B, then it will send the CID of RecordsWrite(A) to DWN B in an EventsGet. DWN B has never seen this message, so I’m actually not sure what it will return, since it’s supposed to return events after the watermark. Does it return no events? Or all events from all time?
@thehenrytsai i went ahead and included functional tests. pls lmk if you'd rather them go elsewhere. |
just realized that we'll need to delete events when a protocol is overwritten as well. Specifically, here. 😭 |
does anyone have any reservations left here? |
The only reservation for me is that the code does a full scan of the event log for every clobber, but fine if we create an issue to improve event deletion by CIDs as a separate item to unblock the merge of this PR. |
@thehenrytsai i'm now storing events in watermark order and cid order. this prevents us from having to scan a tenant's entire event log when deleting by CID. cost incurred is 2x storage for events. the size of each event is requests to change anything else about |
Sounds like a good plan to me, Moe |
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.
🥇 🙏 💯 🎸
First of 3 PRs to implement sync. Introduces an
EventLog
interface along with a concrete level-based implementation. Events are added to the EventLog for:RecordsWrite
RecordsDelete
ProtocolsConfigure
The PR that follows this one will introduce the
EventsGet
interface method