From b33559754df994c4e2e37f1b5b6c8f29ac8f97f1 Mon Sep 17 00:00:00 2001 From: Dmitrii Stepanov Date: Thu, 12 Sep 2024 12:33:12 +0300 Subject: [PATCH] [#1367] fstree: Add size hint for Delete This allow to not to call `os.Stat` if caller already knows data size. Signed-off-by: Dmitrii Stepanov --- .../blobstor/common/delete.go | 1 + .../blobstor/fstree/fstree.go | 2 +- .../blobstor/fstree/fstree_test.go | 84 +++++++++++++------ .../blobstor/fstree/fstree_write_generic.go | 19 +++-- .../blobstor/fstree/fstree_write_linux.go | 25 +++--- pkg/local_object_storage/writecache/flush.go | 4 +- .../writecache/storage.go | 4 +- 7 files changed, 92 insertions(+), 47 deletions(-) diff --git a/pkg/local_object_storage/blobstor/common/delete.go b/pkg/local_object_storage/blobstor/common/delete.go index 1b04eab1a..c19e099cb 100644 --- a/pkg/local_object_storage/blobstor/common/delete.go +++ b/pkg/local_object_storage/blobstor/common/delete.go @@ -8,6 +8,7 @@ import ( type DeletePrm struct { Address oid.Address StorageID []byte + Size uint64 } // DeleteRes groups the resulting values of Delete operation. diff --git a/pkg/local_object_storage/blobstor/fstree/fstree.go b/pkg/local_object_storage/blobstor/fstree/fstree.go index 1c60ec340..057796db2 100644 --- a/pkg/local_object_storage/blobstor/fstree/fstree.go +++ b/pkg/local_object_storage/blobstor/fstree/fstree.go @@ -338,7 +338,7 @@ func (t *FSTree) Delete(ctx context.Context, prm common.DeletePrm) (common.Delet } p := t.treePath(prm.Address) - err = t.writer.removeFile(p) + err = t.writer.removeFile(p, prm.Size) return common.DeleteRes{}, err } diff --git a/pkg/local_object_storage/blobstor/fstree/fstree_test.go b/pkg/local_object_storage/blobstor/fstree/fstree_test.go index f39c7296e..eb2126b6c 100644 --- a/pkg/local_object_storage/blobstor/fstree/fstree_test.go +++ b/pkg/local_object_storage/blobstor/fstree/fstree_test.go @@ -68,34 +68,70 @@ func TestObjectCounter(t *testing.T) { var delPrm common.DeletePrm delPrm.Address = addr - eg, egCtx := errgroup.WithContext(context.Background()) + t.Run("without size hint", func(t *testing.T) { + eg, egCtx := errgroup.WithContext(context.Background()) - eg.Go(func() error { - for range 1_000 { - _, err := fst.Put(egCtx, putPrm) - if err != nil { - return err + eg.Go(func() error { + for range 1_000 { + _, err := fst.Put(egCtx, putPrm) + if err != nil { + return err + } } - } - return nil + return nil + }) + + eg.Go(func() error { + var le logicerr.Logical + for range 1_000 { + _, err := fst.Delete(egCtx, delPrm) + if err != nil && !errors.As(err, &le) { + return err + } + } + return nil + }) + + require.NoError(t, eg.Wait()) + + count, size = counter.CountSize() + realCount, realSize, err := fst.countFiles() + require.NoError(t, err) + require.Equal(t, realCount, count, "real %d, actual %d", realCount, count) + require.Equal(t, realSize, size, "real %d, actual %d", realSize, size) }) - eg.Go(func() error { - var le logicerr.Logical - for range 1_000 { - _, err := fst.Delete(egCtx, delPrm) - if err != nil && !errors.As(err, &le) { - return err + t.Run("with size hint", func(t *testing.T) { + delPrm.Size = uint64(len(putPrm.RawData)) + eg, egCtx := errgroup.WithContext(context.Background()) + + eg.Go(func() error { + for range 1_000 { + _, err := fst.Put(egCtx, putPrm) + if err != nil { + return err + } } - } - return nil + return nil + }) + + eg.Go(func() error { + var le logicerr.Logical + for range 1_000 { + _, err := fst.Delete(egCtx, delPrm) + if err != nil && !errors.As(err, &le) { + return err + } + } + return nil + }) + + require.NoError(t, eg.Wait()) + + count, size = counter.CountSize() + realCount, realSize, err := fst.countFiles() + require.NoError(t, err) + require.Equal(t, realCount, count, "real %d, actual %d", realCount, count) + require.Equal(t, realSize, size, "real %d, actual %d", realSize, size) }) - - require.NoError(t, eg.Wait()) - - count, size = counter.CountSize() - realCount, realSize, err := fst.countFiles() - require.NoError(t, err) - require.Equal(t, realCount, count, "real %d, actual %d", realCount, count) - require.Equal(t, realSize, size, "real %d, actual %d", realSize, size) } diff --git a/pkg/local_object_storage/blobstor/fstree/fstree_write_generic.go b/pkg/local_object_storage/blobstor/fstree/fstree_write_generic.go index 801fc4a22..4110ba7d7 100644 --- a/pkg/local_object_storage/blobstor/fstree/fstree_write_generic.go +++ b/pkg/local_object_storage/blobstor/fstree/fstree_write_generic.go @@ -16,7 +16,7 @@ import ( type writer interface { writeData(string, []byte) error - removeFile(string) error + removeFile(string, uint64) error } type genericWriter struct { @@ -107,10 +107,10 @@ func (w *genericWriter) writeFile(p string, data []byte) error { return err } -func (w *genericWriter) removeFile(p string) error { +func (w *genericWriter) removeFile(p string, size uint64) error { var err error if w.fileCounterEnabled { - err = w.removeWithCounter(p) + err = w.removeWithCounter(p, size) } else { err = os.Remove(p) } @@ -121,18 +121,21 @@ func (w *genericWriter) removeFile(p string) error { return err } -func (w *genericWriter) removeWithCounter(p string) error { +func (w *genericWriter) removeWithCounter(p string, size uint64) error { w.fileGuard.Lock(p) defer w.fileGuard.Unlock(p) - stat, err := os.Stat(p) - if err != nil { - return err + if size == 0 { + stat, err := os.Stat(p) + if err != nil { + return err + } + size = uint64(stat.Size()) } if err := os.Remove(p); err != nil { return err } - w.fileCounter.Dec(uint64(stat.Size())) + w.fileCounter.Dec(uint64(size)) return nil } diff --git a/pkg/local_object_storage/blobstor/fstree/fstree_write_linux.go b/pkg/local_object_storage/blobstor/fstree/fstree_write_linux.go index 3127579ac..3561c616b 100644 --- a/pkg/local_object_storage/blobstor/fstree/fstree_write_linux.go +++ b/pkg/local_object_storage/blobstor/fstree/fstree_write_linux.go @@ -91,25 +91,30 @@ func (w *linuxWriter) writeFile(p string, data []byte) error { return errClose } -func (w *linuxWriter) removeFile(p string) error { +func (w *linuxWriter) removeFile(p string, size uint64) error { if w.fileCounterEnabled { w.fileGuard.Lock(p) defer w.fileGuard.Unlock(p) - } - var stat unix.Stat_t - err := unix.Stat(p, &stat) - if err != nil { - if err == unix.ENOENT { - return logicerr.Wrap(new(apistatus.ObjectNotFound)) + + if size == 0 { + var stat unix.Stat_t + err := unix.Stat(p, &stat) + if err != nil { + if err == unix.ENOENT { + return logicerr.Wrap(new(apistatus.ObjectNotFound)) + } + return err + } + size = uint64(stat.Size) } - return err } - err = unix.Unlink(p) + + err := unix.Unlink(p) if err != nil && err == unix.ENOENT { return logicerr.Wrap(new(apistatus.ObjectNotFound)) } if err == nil { - w.fileCounter.Dec(uint64(stat.Size)) + w.fileCounter.Dec(uint64(size)) } return err } diff --git a/pkg/local_object_storage/writecache/flush.go b/pkg/local_object_storage/writecache/flush.go index 83933375b..bfa6aacb0 100644 --- a/pkg/local_object_storage/writecache/flush.go +++ b/pkg/local_object_storage/writecache/flush.go @@ -123,7 +123,7 @@ func (c *cache) flushIfAnObjectExistsWorker(ctx context.Context, objInfo objectI return } - c.deleteFromDisk(ctx, objInfo.addr) + c.deleteFromDisk(ctx, objInfo.addr, uint64(len(res.RawData))) } func (c *cache) reportFlushError(msg string, addr string, err error) { @@ -157,7 +157,7 @@ func (c *cache) flushFSTree(ctx context.Context, ignoreErrors bool) error { return err } - c.deleteFromDisk(ctx, e.Address) + c.deleteFromDisk(ctx, e.Address, uint64(len(e.ObjectData))) return nil } diff --git a/pkg/local_object_storage/writecache/storage.go b/pkg/local_object_storage/writecache/storage.go index 6aface7a5..2e52e5b20 100644 --- a/pkg/local_object_storage/writecache/storage.go +++ b/pkg/local_object_storage/writecache/storage.go @@ -40,8 +40,8 @@ func (c *cache) openStore(mod mode.ComponentMode) error { return nil } -func (c *cache) deleteFromDisk(ctx context.Context, addr oid.Address) { - _, err := c.fsTree.Delete(ctx, common.DeletePrm{Address: addr}) +func (c *cache) deleteFromDisk(ctx context.Context, addr oid.Address, size uint64) { + _, err := c.fsTree.Delete(ctx, common.DeletePrm{Address: addr, Size: size}) if err != nil && !client.IsErrObjectNotFound(err) { c.log.Error(logs.WritecacheCantRemoveObjectFromWritecache, zap.Error(err)) } else if err == nil {