From b5049d541f0e287e7e00cb85110b2e6ac88fbe77 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Tue, 19 Jan 2021 10:01:06 -0800 Subject: [PATCH] fix: reduce an extra readdir() attempted on non-legacy setups (#11301) to verify moving content and preserving legacy content, we have way to detect the objects through readdir() this path is not necessary for most common cases on newer setups, avoid readdir() to save multiple system calls. also fix the CheckFile behavior for most common use case i.e without legacy format. --- cmd/xl-storage.go | 48 ++++++++++++++++++++++++------------------ cmd/xl-storage_test.go | 2 +- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/cmd/xl-storage.go b/cmd/xl-storage.go index edd537965..0e55207eb 100644 --- a/cmd/xl-storage.go +++ b/cmd/xl-storage.go @@ -1808,16 +1808,18 @@ func (s *xlStorage) CheckFile(ctx context.Context, volume string, path string) e st, _ := os.Lstat(filePath) if st == nil { - if s.formatLegacy { - filePathOld := pathJoin(volumeDir, p, xlStorageFormatFileV1) - if err := checkPathLength(filePathOld); err != nil { - return err - } + if !s.formatLegacy { + return errPathNotFound + } - st, _ = os.Lstat(filePathOld) - if st == nil { - return errPathNotFound - } + filePathOld := pathJoin(volumeDir, p, xlStorageFormatFileV1) + if err := checkPathLength(filePathOld); err != nil { + return err + } + + st, _ = os.Lstat(filePathOld) + if st == nil { + return errPathNotFound } } @@ -2110,18 +2112,24 @@ func (s *xlStorage) RenameData(ctx context.Context, srcVolume, srcPath, dataDir, // It is possible that some drives may not have `xl.meta` file // in such scenarios verify if atleast `part.1` files exist // to verify for legacy version. - currentDataPath := pathJoin(dstVolumeDir, dstPath) - entries, err := readDirN(currentDataPath, 1) - if err != nil && err != errFileNotFound { - return osErrToFileErr(err) - } - for _, entry := range entries { - if entry == xlStorageFormatFile || strings.HasSuffix(entry, slashSeparator) { - continue + if s.formatLegacy { + // We only need this code if we are moving + // from `xl.json` to `xl.meta`, we can avoid + // one extra readdir operation here for all + // new deployments. + currentDataPath := pathJoin(dstVolumeDir, dstPath) + entries, err := readDirN(currentDataPath, 1) + if err != nil && err != errFileNotFound { + return osErrToFileErr(err) } - if strings.HasPrefix(entry, "part.") { - legacyPreserved = true - break + for _, entry := range entries { + if entry == xlStorageFormatFile || strings.HasSuffix(entry, slashSeparator) { + continue + } + if strings.HasPrefix(entry, "part.") { + legacyPreserved = true + break + } } } } diff --git a/cmd/xl-storage_test.go b/cmd/xl-storage_test.go index 3d24ea87f..43a6645b7 100644 --- a/cmd/xl-storage_test.go +++ b/cmd/xl-storage_test.go @@ -1719,7 +1719,7 @@ func TestXLStorageCheckFile(t *testing.T) { for i, testCase := range testCases { if err := xlStorage.CheckFile(context.Background(), testCase.srcVol, testCase.srcPath); err != testCase.expectedErr { - t.Fatalf("TestXLStorage case %d: Expected: \"%s\", got: \"%s\"", i+1, testCase.expectedErr, err) + t.Errorf("TestXLStorage case %d: Expected: \"%s\", got: \"%s\"", i+1, testCase.expectedErr, err) } } }