Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Enhancement: persist commit index in LogStore to accelerate recovery #613
base: main
Are you sure you want to change the base?
Enhancement: persist commit index in LogStore to accelerate recovery #613
Changes from 33 commits
2e5a8a0
ffc6b3b
7383d96
f6295e0
f2ae7a9
ce1895c
ab50a58
4e7e04b
41df55e
400a27d
e2617e8
6daca47
cc09317
fe57b32
20e8701
6f146e1
a8438b0
5e6d8a4
e248f00
2a913ab
7cd6732
92c04a0
8e8ba07
2020cab
2a7d584
bdac45b
ed47a25
ad87d86
30fc43e
e797962
500567f
cfffcb5
560c0b9
8c722fa
300a6e7
8d11a28
1bdf161
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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 think we should also document here that GetCommitIndex need to return 0,nil when no commit index is found in the log.
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.
How would a
GetCommitIndex()
could be implemented in a real store? Would it read the latest stored log and return the commit index associated to it? What if it don't find any because those logs were stored using a store that don't support fast-recovery?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.
For BoltDB, I imagine commit index would be a single KV in a separate bucket from logs so it would just read that and return it.
For WAL I anticipated extending the format slightly so that each commit entry in the log stores the most recently staged commit index and then re-populated that into memory when we open the log an scan it like we do with indexes.
If there is no commit index stored, we should just return
0, nil
which is always safe and has the same behavior as current code I think right?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 agree! I think that should be documented though. Because the API allow erroring on
GetCommitIndex()
it could easily be mistaken as a possible error case.