fix: handle weird drives sporadic read O_DIRECT behavior (#11832)

on freshReads if drive returns errInvalidArgument, we
should simply turn-off DirectIO and read normally, there
are situations in k8s like environments where the drives
behave sporadically in a single deployment and may not
have been implemented properly to handle O_DIRECT for
reads.
This commit is contained in:
Harshavardhana 2021-03-18 20:16:50 -07:00 committed by GitHub
parent be5910b87e
commit b92a220db1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 26 additions and 33 deletions

View File

@ -19,6 +19,7 @@ package cmd
import (
"context"
"errors"
"fmt"
"hash/crc32"
"github.com/minio/minio/cmd/logger"
@ -134,7 +135,8 @@ func readAllFileInfo(ctx context.Context, disks []StorageAPI, bucket, object, ve
errFileVersionNotFound,
errDiskNotFound,
}...) {
logger.LogOnceIf(ctx, err, disks[index].String())
logger.LogOnceIf(ctx, fmt.Errorf("Drive %s returned an error (%w)", disks[index], err),
disks[index].String())
}
}
return err

View File

@ -105,8 +105,6 @@ type xlStorage struct {
rootDisk bool
readODirectSupported bool
diskID string
// Indexes, will be -1 until assigned a set.
@ -274,7 +272,6 @@ func newXLStorage(ep Endpoint) (*xlStorage, error) {
globalSync: env.Get(config.EnvFSOSync, config.EnableOff) == config.EnableOn,
ctx: GlobalContext,
rootDisk: rootDisk,
readODirectSupported: true,
poolIndex: -1,
setIndex: -1,
diskIndex: -1,
@ -301,21 +298,6 @@ func newXLStorage(ep Endpoint) (*xlStorage, error) {
w.Close()
defer os.Remove(filePath)
volumeDir, err := p.getVolDir(minioMetaTmpBucket)
if err != nil {
return p, err
}
// Check if backend is readable, and optionally supports O_DIRECT.
if _, err = p.readAllData(volumeDir, pathJoin(volumeDir, tmpFile), true); err != nil {
if !errors.Is(err, errUnsupportedDisk) {
return p, err
}
// error is unsupported disk, turn-off directIO for reads
logger.LogOnceIf(GlobalContext, fmt.Errorf("Drive %s does not support O_DIRECT for reads, proceeding to use the drive without O_DIRECT", ep), ep.String())
p.readODirectSupported = false
}
// Success.
return p, nil
}
@ -1045,7 +1027,7 @@ func (s *xlStorage) ReadVersion(ctx context.Context, volume, path, versionID str
// - object has maximum of 1 parts
if fi.TransitionStatus == "" && fi.DataDir != "" && fi.Size <= smallFileThreshold && len(fi.Parts) == 1 {
// Enable O_DIRECT optionally only if drive supports it.
requireDirectIO := globalStorageClass.GetDMA() == storageclass.DMAReadWrite && s.readODirectSupported
requireDirectIO := globalStorageClass.GetDMA() == storageclass.DMAReadWrite
partPath := fmt.Sprintf("part.%d", fi.Parts[0].Number)
fi.Data, err = s.readAllData(volumeDir, pathJoin(volumeDir, path, fi.DataDir, partPath), requireDirectIO)
if err != nil {
@ -1125,7 +1107,7 @@ func (s *xlStorage) ReadAll(ctx context.Context, volume string, path string) (bu
return nil, err
}
requireDirectIO := globalStorageClass.GetDMA() == storageclass.DMAReadWrite && s.readODirectSupported
requireDirectIO := globalStorageClass.GetDMA() == storageclass.DMAReadWrite
return s.readAllData(volumeDir, filePath, requireDirectIO)
}
@ -1297,9 +1279,18 @@ func (o *odirectReader) Read(buf []byte) (n int, err error) {
o.buf = *o.bufp
n, err = o.f.Read(o.buf)
if err != nil && err != io.EOF {
if isSysErrInvalidArg(err) {
if err = disk.DisableDirectIO(o.f); err != nil {
o.err = err
return n, err
}
n, err = o.f.Read(o.buf)
}
if err != nil && err != io.EOF {
o.err = err
return n, err
}
}
if n == 0 {
// err is io.EOF
o.err = err
@ -1347,7 +1338,7 @@ func (s *xlStorage) ReadFileStream(ctx context.Context, volume, path string, off
var file *os.File
// O_DIRECT only supported if offset is zero
if offset == 0 && globalStorageClass.GetDMA() == storageclass.DMAReadWrite && s.readODirectSupported {
if offset == 0 && globalStorageClass.GetDMA() == storageclass.DMAReadWrite {
file, err = disk.OpenFileDirectIO(filePath, readMode, 0666)
} else {
// Open the file for reading.
@ -1389,7 +1380,7 @@ func (s *xlStorage) ReadFileStream(ctx context.Context, volume, path string, off
return nil, errIsNotRegular
}
if offset == 0 && globalStorageClass.GetDMA() == storageclass.DMAReadWrite && s.readODirectSupported {
if offset == 0 && globalStorageClass.GetDMA() == storageclass.DMAReadWrite {
or := &odirectReader{file, nil, nil, true, false, s, nil}
if length <= smallFileThreshold {
or = &odirectReader{file, nil, nil, true, true, s, nil}

View File

@ -50,14 +50,14 @@ func TestDeadlineWriter(t *testing.T) {
if err != context.Canceled {
t.Error("DeadlineWriter shouldn't be successful - should return context.Canceled")
}
w = NewDeadlineWriter(&sleepWriter{timeout: 500 * time.Millisecond}, 600*time.Millisecond)
w = NewDeadlineWriter(&sleepWriter{timeout: 100 * time.Millisecond}, 600*time.Millisecond)
n, err := w.Write([]byte("abcd"))
w.Close()
if err != nil {
t.Errorf("DeadlineWriter should succeed but failed with %s", err)
}
if n != 4 {
t.Errorf("DeadlineWriter should succeed but should have only written 0 bytes, but returned %d instead", n)
t.Errorf("DeadlineWriter should succeed but should have only written 4 bytes, but returned %d instead", n)
}
}