From 174f428571e0180df3f23b13fbc259868fe5a48f Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Wed, 1 Jul 2020 10:57:23 -0700 Subject: [PATCH] add additional fdatasync before close() on writes (#9947) --- buildscripts/cross-compile.sh | 2 +- cmd/obdinfo.go | 34 ------------------ cmd/obdinfo_other.go | 36 ++++++++++++++++++- cmd/{obdinfo_freebsd.go => obdinfo_unix.go} | 17 ++++++++- cmd/xl-storage.go | 5 ++- pkg/disk/directio_unsupported.go | 13 +++++-- pkg/disk/fdatasync_linux.go | 38 +++++++++++++++++++++ pkg/disk/fdatasync_unix.go | 29 ++++++++++++++++ pkg/ioutil/ioutil.go | 2 +- 9 files changed, 134 insertions(+), 42 deletions(-) rename cmd/{obdinfo_freebsd.go => obdinfo_unix.go} (71%) create mode 100644 pkg/disk/fdatasync_linux.go create mode 100644 pkg/disk/fdatasync_unix.go diff --git a/buildscripts/cross-compile.sh b/buildscripts/cross-compile.sh index 25436789a..64d8e258a 100755 --- a/buildscripts/cross-compile.sh +++ b/buildscripts/cross-compile.sh @@ -9,7 +9,7 @@ function _init() { export CGO_ENABLED=0 ## List of architectures and OS to test coss compilation. - SUPPORTED_OSARCH="linux/ppc64le linux/arm64 linux/s390x darwin/amd64 freebsd/amd64 windows/amd64 linux/arm linux/386" + SUPPORTED_OSARCH="linux/ppc64le linux/arm64 linux/s390x darwin/amd64 freebsd/amd64 windows/amd64 linux/arm linux/386 netbsd/amd64" } function _build() { diff --git a/cmd/obdinfo.go b/cmd/obdinfo.go index b57d81a99..52fd6aea1 100644 --- a/cmd/obdinfo.go +++ b/cmd/obdinfo.go @@ -27,7 +27,6 @@ import ( "github.com/minio/minio/pkg/disk" "github.com/minio/minio/pkg/madmin" cpuhw "github.com/shirou/gopsutil/cpu" - "github.com/shirou/gopsutil/host" memhw "github.com/shirou/gopsutil/mem" "github.com/shirou/gopsutil/process" ) @@ -369,36 +368,3 @@ func getLocalProcOBD(ctx context.Context, r *http.Request) madmin.ServerProcOBDI Processes: sysProcs, } } - -func getLocalOsInfoOBD(ctx context.Context, r *http.Request) madmin.ServerOsOBDInfo { - addr := r.Host - if globalIsDistErasure { - addr = GetLocalPeer(globalEndpoints) - } - - info, err := host.InfoWithContext(ctx) - if err != nil { - return madmin.ServerOsOBDInfo{ - Addr: addr, - Error: err.Error(), - } - } - - sensors, err := host.SensorsTemperaturesWithContext(ctx) - if err != nil { - return madmin.ServerOsOBDInfo{ - Addr: addr, - Error: err.Error(), - } - } - - // ignore user err, as it cannot be obtained reliably inside containers - users, _ := host.UsersWithContext(ctx) - - return madmin.ServerOsOBDInfo{ - Addr: addr, - Info: info, - Sensors: sensors, - Users: users, - } -} diff --git a/cmd/obdinfo_other.go b/cmd/obdinfo_other.go index 0b323b3af..d176407c4 100644 --- a/cmd/obdinfo_other.go +++ b/cmd/obdinfo_other.go @@ -1,4 +1,4 @@ -// +build !freebsd +// +build !freebsd,!netbsd,!openbsd,!solaris /* * MinIO Cloud Storage, (C) 2020 MinIO, Inc. @@ -26,8 +26,42 @@ import ( "github.com/minio/minio/pkg/madmin" diskhw "github.com/shirou/gopsutil/disk" + "github.com/shirou/gopsutil/host" ) +func getLocalOsInfoOBD(ctx context.Context, r *http.Request) madmin.ServerOsOBDInfo { + addr := r.Host + if globalIsDistErasure { + addr = GetLocalPeer(globalEndpoints) + } + + info, err := host.InfoWithContext(ctx) + if err != nil { + return madmin.ServerOsOBDInfo{ + Addr: addr, + Error: err.Error(), + } + } + + sensors, err := host.SensorsTemperaturesWithContext(ctx) + if err != nil { + return madmin.ServerOsOBDInfo{ + Addr: addr, + Error: err.Error(), + } + } + + // ignore user err, as it cannot be obtained reliably inside containers + users, _ := host.UsersWithContext(ctx) + + return madmin.ServerOsOBDInfo{ + Addr: addr, + Info: info, + Sensors: sensors, + Users: users, + } +} + func getLocalDiskHwOBD(ctx context.Context, r *http.Request) madmin.ServerDiskHwOBDInfo { addr := r.Host if globalIsDistErasure { diff --git a/cmd/obdinfo_freebsd.go b/cmd/obdinfo_unix.go similarity index 71% rename from cmd/obdinfo_freebsd.go rename to cmd/obdinfo_unix.go index 8aab2895f..383f41f60 100644 --- a/cmd/obdinfo_freebsd.go +++ b/cmd/obdinfo_unix.go @@ -1,3 +1,5 @@ +// +build freebsd netbsd openbsd solaris + /* * MinIO Cloud Storage, (C) 2020 MinIO, Inc. * @@ -20,6 +22,7 @@ package cmd import ( "context" "net/http" + "runtime" "github.com/minio/minio/pkg/madmin" ) @@ -32,6 +35,18 @@ func getLocalDiskHwOBD(ctx context.Context, r *http.Request) madmin.ServerDiskHw return madmin.ServerDiskHwOBDInfo{ Addr: addr, - Error: "unsupported platform", + Error: "unsupported platform: " + runtime.GOOS, + } +} + +func getLocalOsInfoOBD(ctx context.Context, r *http.Request) madmin.ServerOsOBDInfo { + addr := r.Host + if globalIsDistErasure { + addr = GetLocalPeer(globalEndpoints) + } + + return madmin.ServerOsOBDInfo{ + Addr: addr, + Error: "unsupported platform: " + runtime.GOOS, } } diff --git a/cmd/xl-storage.go b/cmd/xl-storage.go index 07a3d1d26..4dbef0fae 100644 --- a/cmd/xl-storage.go +++ b/cmd/xl-storage.go @@ -1694,7 +1694,10 @@ func (s *xlStorage) CreateFile(volume, path string, fileSize int64, r io.Reader) return err } - defer w.Close() + defer func() { + disk.Fdatasync(w) // Only interested in flushing the size_t not mtime/atime + w.Close() + }() bufp := s.pool.Get().(*[]byte) defer s.pool.Put(bufp) diff --git a/pkg/disk/directio_unsupported.go b/pkg/disk/directio_unsupported.go index e848b97a2..7241f4560 100644 --- a/pkg/disk/directio_unsupported.go +++ b/pkg/disk/directio_unsupported.go @@ -1,4 +1,4 @@ -// +build !linux,!netbsd,!freebsd,!darwin +// +build !linux,!netbsd,!freebsd,!darwin,!openbsd /* * Minio Cloud Storage, (C) 2019-2020 Minio, Inc. @@ -47,16 +47,23 @@ import ( // see this ZFS-on-Linux commit message: // https://github.com/openzfs/zfs/commit/a584ef26053065f486d46a7335bea222cb03eeea +// OpenFileDirectIO wrapper around os.OpenFile nothing special func OpenFileDirectIO(filePath string, flag int, perm os.FileMode) (*os.File, error) { return os.OpenFile(filePath, flag, perm) } +// DisableDirectIO is a no-op func DisableDirectIO(f *os.File) error { return nil } -// AlignedBlock simply returns an unaligned buffer for systems that do not -// support DirectIO. +// AlignedBlock simply returns an unaligned buffer +// for systems that do not support DirectIO. func AlignedBlock(BlockSize int) []byte { return make([]byte, BlockSize) } + +// Fdatasync is a no-op +func Fdatasync(f *os.File) error { + return nil +} diff --git a/pkg/disk/fdatasync_linux.go b/pkg/disk/fdatasync_linux.go new file mode 100644 index 000000000..d88b1d537 --- /dev/null +++ b/pkg/disk/fdatasync_linux.go @@ -0,0 +1,38 @@ +// +build linux + +/* + * Minio Cloud Storage, (C) 2020 Minio, 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. + */ + +package disk + +import ( + "os" + "syscall" +) + +// Fdatasync - 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. For example, changes to st_atime or st_mtime +// (respectively, time of last access and time of last modification; see inode(7)) +// do not require flushing because they are not necessary for a subsequent data +// read to be handled correctly. On the other hand, a change to the file size +// (st_size, as made by say ftruncate(2)), would require a metadata flush. +// +// The aim of fdatasync() is to reduce disk activity for applications that +// do not require all metadata to be synchronized with the disk. +func Fdatasync(f *os.File) error { + return syscall.Fdatasync(int(f.Fd())) +} diff --git a/pkg/disk/fdatasync_unix.go b/pkg/disk/fdatasync_unix.go new file mode 100644 index 000000000..44d7c6205 --- /dev/null +++ b/pkg/disk/fdatasync_unix.go @@ -0,0 +1,29 @@ +// +build freebsd netbsd openbsd darwin + +/* + * Minio Cloud Storage, (C) 2020 Minio, 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. + */ + +package disk + +import ( + "os" + "syscall" +) + +// Fdatasync is fsync on freebsd/darwin +func Fdatasync(f *os.File) error { + return syscall.Fsync(int(f.Fd())) +} diff --git a/pkg/ioutil/ioutil.go b/pkg/ioutil/ioutil.go index ac77e489d..6c66c3859 100644 --- a/pkg/ioutil/ioutil.go +++ b/pkg/ioutil/ioutil.go @@ -1,5 +1,5 @@ /* - * MinIO Cloud Storage, (C) 2017, 2018 MinIO, Inc. + * MinIO Cloud Storage, (C) 2017-2020 MinIO, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License.