Skip to content
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

avoid ftruncate twice in buffered io #1991

Closed

Conversation

lightmark
Copy link
Contributor

in buffered io, the filesize_ is the real size.

@facebook-github-bot
Copy link
Contributor

@lightmark has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

util/io_posix.cc Outdated
s = IOError(filename_, errno);
} else {
filesize_ = size;
if (use_direct_io()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this logic should happen in WritableFileWriter. It's more straight-forward if File::Truncate() always truncates.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea

@facebook-github-bot
Copy link
Contributor

@lightmark updated the pull request - view changes - changes since last import

@facebook-github-bot
Copy link
Contributor

@lightmark updated the pull request - view changes - changes since last import

@facebook-github-bot
Copy link
Contributor

@lightmark updated the pull request - view changes - changes since last import

@facebook-github-bot
Copy link
Contributor

@lightmark has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@@ -184,11 +184,14 @@ Status WritableFileWriter::Close() {

s = Flush(); // flush cache to OS

Status interim;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now this can happen in line 191?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 198 also needs this

Status interim = writable_file_->Truncate(filesize_);
if (!interim.ok() && s.ok()) {
s = interim;
if (use_direct_io()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make sure it works on Windows.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. WinWritableFile::use_direct_io() exists already

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants