-
Notifications
You must be signed in to change notification settings - Fork 40
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
fix: Correctly parse dockey in broadcast log event. #631
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.
LGTM! Just make sure you remove those Println
before merging.
Oops 😬. |
Codecov Report
@@ Coverage Diff @@
## develop #631 +/- ##
===========================================
+ Coverage 56.97% 57.00% +0.03%
===========================================
Files 122 122
Lines 14644 14636 -8
===========================================
Hits 8343 8343
+ Misses 5586 5578 -8
Partials 715 715
|
Benchmark ResultsSummary
✅ See Better Results...
❌ See Worse Results...
✨ See Unchanged Results...
🐋 See Full Results...
|
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.
5d78323
to
4732897
Compare
Benchmark ResultsSummary
✅ See Better Results...
❌ See Worse Results...
✨ See Unchanged Results...
🐋 See Full Results...
|
Relevant issue(s)
Resolves #508 & Resolves #629
Description
This fixes two outstanding issues, a malformed doc key as a result of manually trying to parse the dockey from the CRDT ID field on the core.CRDT instead of using the proper
core.MakeDocKey
calls. This work also exposed #629.Technically it doesn't 100% resolve this race condition, but it does kick the can down the hill and drastically increases the reliability of the race-condition executing in our favor. This will need to be properly solved by re-engineering the broadcast mechanics. This future work will be tracked in #630.
Tasks
How has this been tested?
Manually,
Specify the platform(s) on which this was tested: