From e8c0a508621911c111d971428e3ac331e270616b Mon Sep 17 00:00:00 2001 From: Klaus Post Date: Mon, 1 May 2023 09:47:49 -0700 Subject: [PATCH] optimization use small blocks up to 64KB (#17107) --- cmd/xl-storage.go | 27 +++++++++++++++++++++------ internal/disk/directio_darwin.go | 3 +++ internal/disk/directio_unix.go | 3 +++ internal/disk/directio_unsupported.go | 3 +++ internal/ioutil/read_file.go | 26 ++++++++++++++++++++++---- 5 files changed, 52 insertions(+), 10 deletions(-) diff --git a/cmd/xl-storage.go b/cmd/xl-storage.go index cee53c24c..f4c8b2021 100644 --- a/cmd/xl-storage.go +++ b/cmd/xl-storage.go @@ -267,7 +267,8 @@ func newXLStorage(ep Endpoint, cleanUp bool) (s *xlStorage, err error) { if err := s.checkODirectDiskSupport(); err == nil { s.oDirect = true } else { - if globalIsErasureSD && errors.Is(err, errUnsupportedDisk) { + // Allow if unsupported platform or single disk. + if errors.Is(err, errUnsupportedDisk) && globalIsErasureSD || !disk.ODirectPlatform { s.oDirect = false } else { return s, err @@ -375,6 +376,10 @@ func (s *xlStorage) Healing() *healingTracker { // with O_DIRECT support, return an error if any and return // errUnsupportedDisk if there is no O_DIRECT support func (s *xlStorage) checkODirectDiskSupport() error { + if !disk.ODirectPlatform { + return errUnsupportedDisk + } + // Check if backend is writable and supports O_DIRECT uuid := mustGetUUID() filePath := pathJoin(s.diskPath, ".writable-check-"+uuid+".tmp") @@ -1498,16 +1503,23 @@ func (s *xlStorage) readAllData(ctx context.Context, volumeDir string, filePath } return nil, dmTime, err } - r := &xioutil.ODirectReader{ - File: f, - SmallFile: true, + r := io.Reader(f) + var dr *xioutil.ODirectReader + if odirectEnabled { + dr = &xioutil.ODirectReader{ + File: f, + SmallFile: true, + } + defer dr.Close() + r = dr + } else { + defer f.Close() } - defer r.Close() // Get size for precise allocation. stat, err := f.Stat() if err != nil { - buf, err = io.ReadAll(r) + buf, err = io.ReadAll(diskHealthReader(ctx, r)) return buf, dmTime, osErrToFileErr(err) } if stat.IsDir() { @@ -1522,6 +1534,9 @@ func (s *xlStorage) readAllData(ctx context.Context, volumeDir string, filePath } else { buf = make([]byte, sz) } + if dr != nil { + dr.SmallFile = sz <= xioutil.BlockSizeSmall*2 + } // Read file... _, err = io.ReadFull(diskHealthReader(ctx, r), buf) diff --git a/internal/disk/directio_darwin.go b/internal/disk/directio_darwin.go index c29efc7cf..d95d0e82a 100644 --- a/internal/disk/directio_darwin.go +++ b/internal/disk/directio_darwin.go @@ -24,6 +24,9 @@ import ( "golang.org/x/sys/unix" ) +// ODirectPlatform indicates if the platform supports O_DIRECT +const ODirectPlatform = true + // OpenFileDirectIO - bypass kernel cache. func OpenFileDirectIO(filePath string, flag int, perm os.FileMode) (*os.File, error) { return directio.OpenFile(filePath, flag, perm) diff --git a/internal/disk/directio_unix.go b/internal/disk/directio_unix.go index 703584ca4..5a436603e 100644 --- a/internal/disk/directio_unix.go +++ b/internal/disk/directio_unix.go @@ -28,6 +28,9 @@ import ( "golang.org/x/sys/unix" ) +// ODirectPlatform indicates if the platform supports O_DIRECT +const ODirectPlatform = true + // OpenFileDirectIO - bypass kernel cache. func OpenFileDirectIO(filePath string, flag int, perm os.FileMode) (*os.File, error) { return directio.OpenFile(filePath, flag, perm) diff --git a/internal/disk/directio_unsupported.go b/internal/disk/directio_unsupported.go index c60b69cdc..6ebf2a357 100644 --- a/internal/disk/directio_unsupported.go +++ b/internal/disk/directio_unsupported.go @@ -24,6 +24,9 @@ import ( "os" ) +// ODirectPlatform indicates if the platform supports O_DIRECT +const ODirectPlatform = false + // OpenBSD, Windows, and illumos do not support O_DIRECT. // On Windows there is no documentation on disabling O_DIRECT. // For these systems we do not attempt to build the 'directio' dependency since diff --git a/internal/ioutil/read_file.go b/internal/ioutil/read_file.go index ab5d35ad4..6559040df 100644 --- a/internal/ioutil/read_file.go +++ b/internal/ioutil/read_file.go @@ -37,9 +37,9 @@ var ( // ReadFileWithFileInfo reads the named file and returns the contents. // A successful call returns err == nil, not err == EOF. // Because ReadFile reads the whole file, it does not treat an EOF from Read -// as an error to be reported, additionall returns os.FileInfo +// as an error to be reported. func ReadFileWithFileInfo(name string) ([]byte, fs.FileInfo, error) { - f, err := OsOpen(name) + f, err := OsOpenFile(name, readMode, 0o666) if err != nil { return nil, nil, err } @@ -62,6 +62,23 @@ func ReadFileWithFileInfo(name string) ([]byte, fs.FileInfo, error) { // // passes NOATIME flag for reads on Unix systems to avoid atime updates. func ReadFile(name string) ([]byte, error) { + if !disk.ODirectPlatform { + // Don't wrap with un-needed buffer. + // Don't use os.ReadFile, since it doesn't pass NO_ATIME when present. + f, err := OsOpenFile(name, readMode, 0o666) + if err != nil { + return nil, err + } + defer f.Close() + st, err := f.Stat() + if err != nil { + return io.ReadAll(f) + } + dst := make([]byte, st.Size()) + _, err = io.ReadFull(f, dst) + return dst, err + } + f, err := OpenFileDirectIO(name, readMode, 0o666) if err != nil { // fallback if there is an error to read @@ -71,6 +88,7 @@ func ReadFile(name string) ([]byte, error) { return nil, err } } + r := &ODirectReader{ File: f, SmallFile: true, @@ -82,8 +100,8 @@ func ReadFile(name string) ([]byte, error) { return io.ReadAll(r) } - // Select bigger blocks when reading at least 50% of a big block. - r.SmallFile = st.Size() <= BlockSizeLarge/2 + // Select bigger blocks when reading would do more than 2 reads. + r.SmallFile = st.Size() <= BlockSizeSmall*2 dst := make([]byte, st.Size()) _, err = io.ReadFull(r, dst)