-
Notifications
You must be signed in to change notification settings - Fork 37.7k
refactor: Open file in FileCommit #28006
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
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
fa9350f
to
624ba04
Compare
Closing for now, until there is a higher need for it. |
Looks like this will make it safer to remove |
🐙 This pull request conflicts with the target branch and needs rebase. |
This is needed for a future commit.
This allows to remove platform-specific code and avoids it being maintained in this codebase. This is a refactor, apart from potentially using one more file descriptor at runtime, due to the same file being opened already earlier. However, the increase in file descriptors will be undone in the next commit.
* This removes the requirement from the call site to use a raw C-style FILE pointer. Now the call site can use any style of file IO. * This makes the edge-case impossible to call FileCommit with nullptr, which would eventually lead to a segfault. * This removes the burden from the call site to first open the file. * This avoids a fseek if the file was opened using FlatFileSeq::Open with a non-zero nPos. * Where the file was already open at a call site, it removes the burden to keep it open for FileCommit. Call sites are now expected to close or flush the file before calling FileCommit. * This removes the requirement from the call site to open the file with the right mode (write). This should be a refactor, because the following corner cases never happen: * FlatFileSeq::Open has a side-effect to create the file when it is missing. Thus, in WriteFilterToDisk, if the first filter overflew the first file, the node would abort. Previously it would create an empty first filter file.
Closing for now. |
This improves
FileCommit
: The patch removes the requirement from the call site to open or obtain a raw C-style FILE pointer. Now the call site can use any style of file IO.There are many other smaller improvements, which are explained in each commit.