-
Notifications
You must be signed in to change notification settings - Fork 20.6k
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
core/rawdb: introduce flush offset in freezer #30392
Merged
fjl
merged 5 commits into
ethereum:master
from
rjl493456442:freezer-index-validation-optimize
Feb 4, 2025
Merged
core/rawdb: introduce flush offset in freezer #30392
fjl
merged 5 commits into
ethereum:master
from
rjl493456442:freezer-index-validation-optimize
Feb 4, 2025
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6ed7a57
to
ddefb09
Compare
28b2490
to
7b3d9bf
Compare
7b3d9bf
to
fa72fb3
Compare
fa72fb3
to
47da6db
Compare
00a251a
to
5d89190
Compare
This needs to be deployed on benchmark machine another time. |
5d89190
to
61b75e0
Compare
9e08f52
to
b4dcb19
Compare
Couple modifications here: - Use switch instead of a condition chain. It seems better to me since the idea is comparing the file size with flushOffset and performing different actions when it's larger, smaller, equal. - Rephrase some messages to add context. Users should be able to infer it's about the freezer from the message. - Rename 'offset' to 'size'.
The flushOffset is a file position, which is usually tracked as int64 in Go. It's better to keep it as the same type to avoid conversion. Overall, this change removes some conversions, and introduces some. The new conversions are in cases where flushOffset is compared with numEntries*indexEntrySize computations.
a5f9a49
to
eb5a449
Compare
fjl
approved these changes
Feb 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
It's a following PR of #29792 to get rid of the data file sync.
This is a non-backward compatible change, which increments the database version from 8 to 9.
This pull request introduces a flushOffset for each freezer table, which tracks the position of the most recently fsync’d item in the index file.
When this offset moves forward, it indicates that all index entries below it, along with their corresponding data items, have been properly persisted to disk. The offset can also be moved backward when truncating from either the head or tail of the file.
Previously, the data file required an explicit fsync after every mutation, which was highly inefficient. With the introduction of the flush offset, the synchronization strategy becomes more flexible, allowing the freezer to sync every 30 seconds instead.
The data items above the flush offset are regarded volatile and callers must to ensure they are recoverable after the unclean shutdown, or explicitly sync the freezer before any proceeding operations.