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

io/ioutil: document that WriteFile does not do a f.Sync #20599

Closed
prashanthpai opened this issue Jun 7, 2017 · 9 comments
Closed

io/ioutil: document that WriteFile does not do a f.Sync #20599

prashanthpai opened this issue Jun 7, 2017 · 9 comments
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@prashanthpai
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.8 linux/amd64

What operating system and processor architecture are you using (go env)?

$ go env
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/ppai/work"
GORACE=""
GOROOT="/home/ppai/go"
GOTOOLDIR="/home/ppai/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build569018824=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

Use ioutil.WriteFile.

What did you expect to see?

I'm sorry if the expectation is wrong but I expected the source of ioutil.WriteFile to have a call to
f.Sync()

What did you see instead?

No call to f.Sync() was made internally

If fixing this is acceptable, I'll be glad to send a small patch 👍

@mvdan
Copy link
Member

mvdan commented Jun 7, 2017

Out of curiosity, why? In general, if you're writing to a file, you want to check for an f.Close() error. As long as you do that, I don't think a call to f.Sync() is necessary.

@dominikh
Copy link
Member

dominikh commented Jun 7, 2017

@mvdan Closing a file doesn't issue a sync. At the same time, there are plenty of files for which you won't care, and syncing unnecessarily will incur a performance cost. Maybe this is a documentation issue and no change to the code should be made.

@mvdan
Copy link
Member

mvdan commented Jun 7, 2017

My bad - I thought closing a file meant syncing it too, but I must have gotten confused. It's a bit bizarre that you could care about close errors but not about sync errors, though.

@dsnet
Copy link
Member

dsnet commented Jun 7, 2017

Some discussion about f.Sync in WriteFile has been discussed in the past. See #17869.

While it may seem reasonable for f.Sync to be called in your use case, it not reasonable to inflict the cost of f.Sync upon every use of it. You can always work around this, by creating the file yourself and writing to it and then calling Sync.

This is indeed a documentation issue.

@dsnet dsnet changed the title io/ioutil: WriteFile() should internally do a f.Sync() io/ioutil: document that WriteFile does not do a f.Sync Jun 7, 2017
@bradfitz bradfitz added this to the Go1.10 milestone Jun 7, 2017
@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 7, 2017
@bradfitz
Copy link
Contributor

bradfitz commented Jun 7, 2017

We can document the lack of fsync for Go 1.10 unless somebody sends a change for Go 1.9 which would be fine too.

@ianlancetaylor
Copy link
Member

That is an odd sort of documentation to write. There are all kinds of functions that do not call os.File.Sync. Perhaps my expectations are wrong, but I would expect the reverse: I would expect that if ioutil.WriteFile called Sync, that that would be called out in the documentation. I would not expect the documentation to explicitly say that it does not call Sync.

@bradfitz
Copy link
Contributor

bradfitz commented Jun 7, 2017

I would agree, but this keeps coming up, so it makes me wonder if my expectations are wrong.

But I'm also fine not doing anything here again.

@robpike
Copy link
Contributor

robpike commented Jun 8, 2017

Lots of things don't call sync. This is not one worth singling out. I say stet.

@bradfitz
Copy link
Contributor

bradfitz commented Jun 8, 2017

Okay, fair enough. Closing.

@bradfitz bradfitz closed this as completed Jun 8, 2017
mrodm added a commit to mrodm/pouch that referenced this issue Feb 1, 2018
iouti.WriteFile function does not call os.File.Sync internally
(golang/go#20599), so the code has
been updated to forced a sync after each file written by an update
mrodm added a commit to mrodm/pouch that referenced this issue Feb 7, 2018
iouti.WriteFile function does not call os.File.Sync internally
(golang/go#20599), so the code has
been updated to forced a sync after each file written by an update
mrodm added a commit to mrodm/pouch that referenced this issue Feb 13, 2018
iouti.WriteFile function does not call os.File.Sync internally
(golang/go#20599), so the code has
been updated to forced a sync after each file written by an update
@golang golang locked and limited conversation to collaborators Jun 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

8 participants