From 8b6de5f85deb37400d726090355a4834d6b3f0e4 Mon Sep 17 00:00:00 2001 From: Ajit Yagaty Date: Mon, 18 Apr 2016 00:25:21 -0700 Subject: [PATCH] fileutil: Sync on HFS/OSX needs to be handled differently. A call file.Sync on OSX doesn't guarantee actual persistence on physical drive media as the data can be cached in physical drive's buffers. Hence calls to file.Sync need to be replaced with fcntl(F_FULLFSYNC). --- etcdctl/ctlv3/command/snapshot_command.go | 3 +- pkg/fileutil/sync.go | 11 ++++--- pkg/fileutil/sync_darwin.go | 40 +++++++++++++++++++++++ pkg/fileutil/sync_linux.go | 5 +++ pkg/ioutil/util.go | 4 ++- snap/db.go | 2 +- wal/repair.go | 2 +- 7 files changed, 59 insertions(+), 8 deletions(-) create mode 100644 pkg/fileutil/sync_darwin.go diff --git a/etcdctl/ctlv3/command/snapshot_command.go b/etcdctl/ctlv3/command/snapshot_command.go index 586344384..d2554f727 100644 --- a/etcdctl/ctlv3/command/snapshot_command.go +++ b/etcdctl/ctlv3/command/snapshot_command.go @@ -28,6 +28,7 @@ import ( "github.com/coreos/etcd/etcdserver" "github.com/coreos/etcd/etcdserver/etcdserverpb" "github.com/coreos/etcd/etcdserver/membership" + "github.com/coreos/etcd/pkg/fileutil" "github.com/coreos/etcd/pkg/types" "github.com/coreos/etcd/raft" "github.com/coreos/etcd/raft/raftpb" @@ -121,7 +122,7 @@ func snapshotSaveCommandFunc(cmd *cobra.Command, args []string) { ExitWithError(ExitInterrupted, rerr) } - f.Sync() + fileutil.Fsync(f) if rerr := os.Rename(partpath, path); rerr != nil { exiterr := fmt.Errorf("could not rename %s to %s (%v)", partpath, path, rerr) diff --git a/pkg/fileutil/sync.go b/pkg/fileutil/sync.go index cd7fff08f..a089154ef 100644 --- a/pkg/fileutil/sync.go +++ b/pkg/fileutil/sync.go @@ -12,15 +12,18 @@ // See the License for the specific language governing permissions and // limitations under the License. -// +build !linux +// +build !linux,!darwin package fileutil import "os" -// Fdatasync is similar to fsync(), but does not flush modified metadata -// unless that metadata is needed in order to allow a subsequent data retrieval -// to be correctly handled. +// Fsync is a wrapper around file.Sync(). Special handling is needed on darwin platform. +func Fsync(f *os.File) error { + return f.Sync() +} + +// Fdatasync is a wrapper around file.Sync(). Special handling is needed on linux platform. func Fdatasync(f *os.File) error { return f.Sync() } diff --git a/pkg/fileutil/sync_darwin.go b/pkg/fileutil/sync_darwin.go new file mode 100644 index 000000000..07423ffbb --- /dev/null +++ b/pkg/fileutil/sync_darwin.go @@ -0,0 +1,40 @@ +// Copyright 2016 CoreOS, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// +build darwin + +package fileutil + +import ( + "os" + "syscall" +) + +// Fsync on HFS/OSX flushes the data on to the physical drive but the drive +// may not write it to the persistent media for quite sometime and it may be +// written in out-of-order sequence. Using F_FULLFSYNC ensures that the +// physical drive's buffer will also get flushed to the media. +func Fsync(f *os.File) error { + _, _, errno := syscall.Syscall(syscall.SYS_FCNTL, f.Fd(), uintptr(syscall.F_FULLFSYNC), uintptr(0)) + if errno == 0 { + return nil + } + return errno +} + +// Fdatasync on darwin platform invokes fcntl(F_FULLFSYNC) for actual persistence +// on physical drive media. +func Fdatasync(f *os.File) error { + return Fsync(f) +} diff --git a/pkg/fileutil/sync_linux.go b/pkg/fileutil/sync_linux.go index 14c4b4808..b923996f9 100644 --- a/pkg/fileutil/sync_linux.go +++ b/pkg/fileutil/sync_linux.go @@ -21,6 +21,11 @@ import ( "syscall" ) +// Fsync is a wrapper around file.Sync(). Special handling is needed on darwin platform. +func Fsync(f *os.File) error { + return f.Sync() +} + // Fdatasync is similar to fsync(), but does not flush modified metadata // unless that metadata is needed in order to allow a subsequent data retrieval // to be correctly handled. diff --git a/pkg/ioutil/util.go b/pkg/ioutil/util.go index 9acc79830..6098da282 100644 --- a/pkg/ioutil/util.go +++ b/pkg/ioutil/util.go @@ -17,6 +17,8 @@ package ioutil import ( "io" "os" + + "github.com/coreos/etcd/pkg/fileutil" ) // WriteAndSyncFile behaves just like ioutil.WriteFile in the standard library, @@ -32,7 +34,7 @@ func WriteAndSyncFile(filename string, data []byte, perm os.FileMode) error { err = io.ErrShortWrite } if err == nil { - err = f.Sync() + err = fileutil.Fsync(f) } if err1 := f.Close(); err == nil { err = err1 diff --git a/snap/db.go b/snap/db.go index ca68837cb..4a2083fe5 100644 --- a/snap/db.go +++ b/snap/db.go @@ -34,7 +34,7 @@ func (s *Snapshotter) SaveDBFrom(r io.Reader, id uint64) error { var n int64 n, err = io.Copy(f, r) if err == nil { - err = f.Sync() + err = fileutil.Fsync(f) } f.Close() if err != nil { diff --git a/wal/repair.go b/wal/repair.go index eceebff3c..56030e7b9 100644 --- a/wal/repair.go +++ b/wal/repair.go @@ -78,7 +78,7 @@ func Repair(dirpath string) bool { plog.Errorf("could not repair %v, failed to truncate file", f.Name()) return false } - if err = f.Sync(); err != nil { + if err = fileutil.Fsync(f); err != nil { plog.Errorf("could not repair %v, failed to sync file", f.Name()) return false }