From 124e28578cfa659f0e2e837f474a39f7e812ea8d Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Fri, 25 Aug 2023 07:58:11 -0700 Subject: [PATCH] remove strict persistence requirements for List() .metacache objects (#17917) .metacache objects are transient in nature, and are better left to use page-cache effectively to avoid using more IOPs on the disks. this allows for incoming calls to be not taxed heavily due to multiple large batch listings. --- cmd/erasure-object.go | 10 ++++++--- cmd/metacache-set.go | 2 +- cmd/naughty-disk_test.go | 4 ++-- cmd/storage-interface.go | 4 ++-- cmd/storage-rest-client.go | 3 ++- cmd/storage-rest-common.go | 1 + cmd/storage-rest-server.go | 3 ++- cmd/xl-storage-disk-id-check.go | 4 ++-- cmd/xl-storage.go | 40 ++++++++++++++++++--------------- 9 files changed, 41 insertions(+), 30 deletions(-) diff --git a/cmd/erasure-object.go b/cmd/erasure-object.go index 4891dd29c..b1ef3fa54 100644 --- a/cmd/erasure-object.go +++ b/cmd/erasure-object.go @@ -1969,8 +1969,7 @@ func (er erasureObjects) PutObjectTags(ctx context.Context, bucket, object strin return fi.ToObjectInfo(bucket, object, opts.Versioned || opts.VersionSuspended), nil } -// updateObjectMeta will update the metadata of a file. -func (er erasureObjects) updateObjectMeta(ctx context.Context, bucket, object string, fi FileInfo, onlineDisks []StorageAPI) error { +func (er erasureObjects) updateObjectMetaWithOpts(ctx context.Context, bucket, object string, fi FileInfo, onlineDisks []StorageAPI, opts UpdateMetadataOpts) error { if len(fi.Metadata) == 0 { return nil } @@ -1984,7 +1983,7 @@ func (er erasureObjects) updateObjectMeta(ctx context.Context, bucket, object st if onlineDisks[index] == nil { return errDiskNotFound } - return onlineDisks[index].UpdateMetadata(ctx, bucket, object, fi) + return onlineDisks[index].UpdateMetadata(ctx, bucket, object, fi, opts) }, index) } @@ -1994,6 +1993,11 @@ func (er erasureObjects) updateObjectMeta(ctx context.Context, bucket, object st return reduceWriteQuorumErrs(ctx, mErrs, objectOpIgnoredErrs, er.defaultWQuorum()) } +// updateObjectMeta will update the metadata of a file. +func (er erasureObjects) updateObjectMeta(ctx context.Context, bucket, object string, fi FileInfo, onlineDisks []StorageAPI) error { + return er.updateObjectMetaWithOpts(ctx, bucket, object, fi, onlineDisks, UpdateMetadataOpts{}) +} + // DeleteObjectTags - delete object tags from an existing object func (er erasureObjects) DeleteObjectTags(ctx context.Context, bucket, object string, opts ObjectOptions) (ObjectInfo, error) { return er.PutObjectTags(ctx, bucket, object, "", opts) diff --git a/cmd/metacache-set.go b/cmd/metacache-set.go index 7e52793b9..e3bb37d5f 100644 --- a/cmd/metacache-set.go +++ b/cmd/metacache-set.go @@ -785,7 +785,7 @@ func (er *erasureObjects) saveMetaCacheStream(ctx context.Context, mc *metaCache for k, v := range meta { fi.Metadata[k] = v } - err := er.updateObjectMeta(ctx, minioMetaBucket, o.objectPath(0), fi, er.getDisks()) + err := er.updateObjectMetaWithOpts(ctx, minioMetaBucket, o.objectPath(0), fi, er.getDisks(), UpdateMetadataOpts{NoPersistence: true}) if err == nil { break } diff --git a/cmd/naughty-disk_test.go b/cmd/naughty-disk_test.go index f629a5015..1a32eeb28 100644 --- a/cmd/naughty-disk_test.go +++ b/cmd/naughty-disk_test.go @@ -246,11 +246,11 @@ func (d *naughtyDisk) WriteMetadata(ctx context.Context, volume, path string, fi return d.disk.WriteMetadata(ctx, volume, path, fi) } -func (d *naughtyDisk) UpdateMetadata(ctx context.Context, volume, path string, fi FileInfo) (err error) { +func (d *naughtyDisk) UpdateMetadata(ctx context.Context, volume, path string, fi FileInfo, opts UpdateMetadataOpts) (err error) { if err := d.calcError(); err != nil { return err } - return d.disk.UpdateMetadata(ctx, volume, path, fi) + return d.disk.UpdateMetadata(ctx, volume, path, fi, opts) } func (d *naughtyDisk) DeleteVersion(ctx context.Context, volume, path string, fi FileInfo, forceDelMarker bool) (err error) { diff --git a/cmd/storage-interface.go b/cmd/storage-interface.go index 69bb0f76a..1ac1cc2d7 100644 --- a/cmd/storage-interface.go +++ b/cmd/storage-interface.go @@ -82,7 +82,7 @@ type StorageAPI interface { DeleteVersion(ctx context.Context, volume, path string, fi FileInfo, forceDelMarker bool) error DeleteVersions(ctx context.Context, volume string, versions []FileInfoVersions) []error WriteMetadata(ctx context.Context, volume, path string, fi FileInfo) error - UpdateMetadata(ctx context.Context, volume, path string, fi FileInfo) error + UpdateMetadata(ctx context.Context, volume, path string, fi FileInfo, opts UpdateMetadataOpts) error ReadVersion(ctx context.Context, volume, path, versionID string, readData bool) (FileInfo, error) ReadXL(ctx context.Context, volume, path string, readData bool) (RawFileInfo, error) RenameData(ctx context.Context, srcVolume, srcPath string, fi FileInfo, dstVolume, dstPath string) (uint64, error) @@ -252,7 +252,7 @@ func (p *unrecognizedDisk) DeleteVersion(ctx context.Context, volume, path strin return errDiskNotFound } -func (p *unrecognizedDisk) UpdateMetadata(ctx context.Context, volume, path string, fi FileInfo) (err error) { +func (p *unrecognizedDisk) UpdateMetadata(ctx context.Context, volume, path string, fi FileInfo, opts UpdateMetadataOpts) (err error) { return errDiskNotFound } diff --git a/cmd/storage-rest-client.go b/cmd/storage-rest-client.go index 56d542f65..b565cc6ee 100644 --- a/cmd/storage-rest-client.go +++ b/cmd/storage-rest-client.go @@ -406,10 +406,11 @@ func (client *storageRESTClient) WriteMetadata(ctx context.Context, volume, path return err } -func (client *storageRESTClient) UpdateMetadata(ctx context.Context, volume, path string, fi FileInfo) error { +func (client *storageRESTClient) UpdateMetadata(ctx context.Context, volume, path string, fi FileInfo, opts UpdateMetadataOpts) error { values := make(url.Values) values.Set(storageRESTVolume, volume) values.Set(storageRESTFilePath, path) + values.Set(storageRESTNoPersistence, strconv.FormatBool(opts.NoPersistence)) var reader bytes.Buffer if err := msgp.Encode(&reader, &fi); err != nil { diff --git a/cmd/storage-rest-common.go b/cmd/storage-rest-common.go index 086d98928..326b80fff 100644 --- a/cmd/storage-rest-common.go +++ b/cmd/storage-rest-common.go @@ -84,4 +84,5 @@ const ( storageRESTGlob = "glob" storageRESTScanMode = "scan-mode" storageRESTMetrics = "metrics" + storageRESTNoPersistence = "no-persistence" ) diff --git a/cmd/storage-rest-server.go b/cmd/storage-rest-server.go index 73c016ab5..2dbe4398b 100644 --- a/cmd/storage-rest-server.go +++ b/cmd/storage-rest-server.go @@ -442,6 +442,7 @@ func (s *storageRESTServer) UpdateMetadataHandler(w http.ResponseWriter, r *http } volume := r.Form.Get(storageRESTVolume) filePath := r.Form.Get(storageRESTFilePath) + noPersistence := r.Form.Get(storageRESTNoPersistence) == "true" if r.ContentLength < 0 { s.writeErrorResponse(w, errInvalidArgument) @@ -454,7 +455,7 @@ func (s *storageRESTServer) UpdateMetadataHandler(w http.ResponseWriter, r *http return } - err := s.storage.UpdateMetadata(r.Context(), volume, filePath, fi) + err := s.storage.UpdateMetadata(r.Context(), volume, filePath, fi, UpdateMetadataOpts{NoPersistence: noPersistence}) if err != nil { s.writeErrorResponse(w, err) } diff --git a/cmd/xl-storage-disk-id-check.go b/cmd/xl-storage-disk-id-check.go index 16724994e..95223444f 100644 --- a/cmd/xl-storage-disk-id-check.go +++ b/cmd/xl-storage-disk-id-check.go @@ -526,7 +526,7 @@ func (p *xlStorageDiskIDCheck) DeleteVersion(ctx context.Context, volume, path s return w.Run(func() error { return p.storage.DeleteVersion(ctx, volume, path, fi, forceDelMarker) }) } -func (p *xlStorageDiskIDCheck) UpdateMetadata(ctx context.Context, volume, path string, fi FileInfo) (err error) { +func (p *xlStorageDiskIDCheck) UpdateMetadata(ctx context.Context, volume, path string, fi FileInfo, opts UpdateMetadataOpts) (err error) { ctx, done, err := p.TrackDiskHealth(ctx, storageMetricUpdateMetadata, volume, path) if err != nil { return err @@ -534,7 +534,7 @@ func (p *xlStorageDiskIDCheck) UpdateMetadata(ctx context.Context, volume, path defer done(&err) w := xioutil.NewDeadlineWorker(diskMaxTimeout) - return w.Run(func() error { return p.storage.UpdateMetadata(ctx, volume, path, fi) }) + return w.Run(func() error { return p.storage.UpdateMetadata(ctx, volume, path, fi, opts) }) } func (p *xlStorageDiskIDCheck) WriteMetadata(ctx context.Context, volume, path string, fi FileInfo) (err error) { diff --git a/cmd/xl-storage.go b/cmd/xl-storage.go index cb1fa6603..9772afc82 100644 --- a/cmd/xl-storage.go +++ b/cmd/xl-storage.go @@ -964,19 +964,23 @@ func (s *xlStorage) ListDir(ctx context.Context, volume, dirPath string, count i } func (s *xlStorage) deleteVersions(ctx context.Context, volume, path string, fis ...FileInfo) error { + volumeDir, err := s.getVolDir(volume) + if err != nil { + return err + } + var legacyJSON bool - buf, err := s.ReadAll(ctx, volume, pathJoin(path, xlStorageFormatFile)) + buf, _, err := s.readAllData(ctx, volumeDir, pathJoin(volumeDir, path, xlStorageFormatFile), false) if err != nil { if !errors.Is(err, errFileNotFound) { return err } - metaDataPoolPut(buf) // Never used, return it s.RLock() legacy := s.formatLegacy s.RUnlock() if legacy { - buf, err = s.ReadAll(ctx, volume, pathJoin(path, xlStorageFormatFileV1)) + buf, _, err = s.readAllData(ctx, volumeDir, pathJoin(volumeDir, path, xlStorageFormatFileV1), false) if err != nil { return err } @@ -988,11 +992,6 @@ func (s *xlStorage) deleteVersions(ctx context.Context, volume, path string, fis return errFileNotFound } - volumeDir, err := s.getVolDir(volume) - if err != nil { - return err - } - if legacyJSON { // Delete the meta file, if there are no more versions the // top level parent is automatically removed. @@ -1210,8 +1209,13 @@ func (s *xlStorage) DeleteVersion(ctx context.Context, volume, path string, fi F return s.deleteFile(volumeDir, filePath, true, false) } +// UpdateMetadataOpts provides an optional input to indicate if xl.meta updates need to be fully synced to disk. +type UpdateMetadataOpts struct { + NoPersistence bool +} + // Updates only metadata for a given version. -func (s *xlStorage) UpdateMetadata(ctx context.Context, volume, path string, fi FileInfo) error { +func (s *xlStorage) UpdateMetadata(ctx context.Context, volume, path string, fi FileInfo, opts UpdateMetadataOpts) error { if len(fi.Metadata) == 0 { return errInvalidArgument } @@ -1247,7 +1251,7 @@ func (s *xlStorage) UpdateMetadata(ctx context.Context, volume, path string, fi } defer metaDataPoolPut(wbuf) - return s.WriteAll(ctx, volume, pathJoin(path, xlStorageFormatFile), wbuf) + return s.writeAll(ctx, volume, pathJoin(path, xlStorageFormatFile), wbuf, !opts.NoPersistence) } // WriteMetadata - writes FileInfo metadata for path at `xl.meta` @@ -1365,7 +1369,7 @@ func (s *xlStorage) renameLegacyMetadata(volumeDir, path string) (err error) { func (s *xlStorage) readRaw(ctx context.Context, volumeDir, filePath string, readData bool) (buf []byte, dmTime time.Time, err error) { if readData { - buf, dmTime, err = s.readAllData(ctx, volumeDir, pathJoin(filePath, xlStorageFormatFile)) + buf, dmTime, err = s.readAllData(ctx, volumeDir, pathJoin(filePath, xlStorageFormatFile), true) } else { buf, dmTime, err = s.readMetadataWithDMTime(ctx, pathJoin(filePath, xlStorageFormatFile)) if err != nil { @@ -1380,7 +1384,7 @@ func (s *xlStorage) readRaw(ctx context.Context, volumeDir, filePath string, rea if err != nil { if err == errFileNotFound { - buf, dmTime, err = s.readAllData(ctx, volumeDir, pathJoin(filePath, xlStorageFormatFileV1)) + buf, dmTime, err = s.readAllData(ctx, volumeDir, pathJoin(filePath, xlStorageFormatFileV1), true) if err != nil { return nil, time.Time{}, err } @@ -1488,7 +1492,7 @@ func (s *xlStorage) ReadVersion(ctx context.Context, volume, path, versionID str len(fi.Parts) == 1 { partPath := fmt.Sprintf("part.%d", fi.Parts[0].Number) dataPath := pathJoin(volumeDir, path, fi.DataDir, partPath) - fi.Data, _, err = s.readAllData(ctx, volumeDir, dataPath) + fi.Data, _, err = s.readAllData(ctx, volumeDir, dataPath, true) if err != nil { return FileInfo{}, err } @@ -1498,14 +1502,14 @@ func (s *xlStorage) ReadVersion(ctx context.Context, volume, path, versionID str return fi, nil } -func (s *xlStorage) readAllData(ctx context.Context, volumeDir string, filePath string) (buf []byte, dmTime time.Time, err error) { +func (s *xlStorage) readAllData(ctx context.Context, volumeDir string, filePath string, sync bool) (buf []byte, dmTime time.Time, err error) { if contextCanceled(ctx) { return nil, time.Time{}, ctx.Err() } odirectEnabled := globalAPIConfig.odirectEnabled() && s.oDirect var f *os.File - if odirectEnabled { + if odirectEnabled && sync { f, err = OpenFileDirectIO(filePath, readMode, 0o666) } else { f, err = OpenFile(filePath, readMode, 0o666) @@ -1606,7 +1610,7 @@ func (s *xlStorage) ReadAll(ctx context.Context, volume string, path string) (bu return nil, err } - buf, _, err = s.readAllData(ctx, volumeDir, filePath) + buf, _, err = s.readAllData(ctx, volumeDir, filePath, true) return buf, err } @@ -2705,7 +2709,7 @@ func (s *xlStorage) ReadMultiple(ctx context.Context, req ReadMultipleReq, resp if req.MetadataOnly { data, mt, err = s.readMetadataWithDMTime(ctx, fullPath) } else { - data, mt, err = s.readAllData(ctx, volumeDir, fullPath) + data, mt, err = s.readAllData(ctx, volumeDir, fullPath, false) } return err }); err != nil { @@ -2798,7 +2802,7 @@ func (s *xlStorage) CleanAbandonedData(ctx context.Context, volume string, path } baseDir := pathJoin(volumeDir, path+slashSeparator) metaPath := pathutil.Join(baseDir, xlStorageFormatFile) - buf, _, err := s.readAllData(ctx, volumeDir, metaPath) + buf, _, err := s.readAllData(ctx, volumeDir, metaPath, false) if err != nil { return err }