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
core/rawdb: implement sequential reads in freezer_table #23117
core/rawdb: implement sequential reads in freezer_table #23117
Changes from 3 commits
441bdd4
d8a24b4
ff8267c
d6b40b8
821c696
4e5b6f9
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.
"the given indexEntry, which are assumed to be sequential." made me do 5 takes until I understood what it means. Perhaps we could reformulate it like
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.
Seems weird to call this startIndex, since we use it as
index
for every item. Also considerindex := new(indexEntry)
. No need for thevar
form.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'm not really following. Isn't the offset already zero if it's the first item? I also don't understand why the file number is assigned?
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.
Alternatively, isn't this the case for all items that start a new file, not just
from == 0
?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.
Or, is the point that "indices[0].filenum` might not exist at all (since it's a deleted file), vs. anything else exists and we can "read and see the data overflown"?
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's more or less what the comment says above. So each index is a pair of
filenum, offset
. Like this:However, the
0
th index will always be| 000 | 000 | 000 |
, right?Therefore, back in the day, we decided to use this quirk to implement deletability. So if we want to delete the file
0
and1
, we need to store the item-offset somewhere. So we store that in theoffset
(which otherwise is the byte-offset within the file -- but we know the first item in a file always has byte offset0
).I think you're right though, that we wouldn't really need to set the filenum. However, as you can see in the previous code, for item
N
, marked byindexA
andindexB
, the actual data is always stored in the file given byindexB
. The two may differ, if the data crosses a file boundary.In this case, we want to return to the caller info about where to start reading, and where to stop. Therefore, doing this conversion here simplifies for the calling code, which can just treat the indices as direct data information without dealing with the intricacies of the data indexing scheme.
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.
Nitpick, but instead of max, I'd call this count