From 1fc95f6ed88dae74d5e1f8fa779de9d7d594760b Mon Sep 17 00:00:00 2001 From: Dmitrii Stepanov Date: Wed, 22 Jan 2025 11:36:34 +0300 Subject: [PATCH] Revert "[#1367] fstree: Add size hint for Delete" This reverts commit b33559754df994c4e2e37f1b5b6c8f29ac8f97f1. --- .../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 | 27 +++--- pkg/local_object_storage/writecache/flush.go | 4 +- .../writecache/storage.go | 4 +- 7 files changed, 48 insertions(+), 93 deletions(-) diff --git a/pkg/local_object_storage/blobstor/common/delete.go b/pkg/local_object_storage/blobstor/common/delete.go index c19e099cb..1b04eab1a 100644 --- a/pkg/local_object_storage/blobstor/common/delete.go +++ b/pkg/local_object_storage/blobstor/common/delete.go @@ -8,7 +8,6 @@ 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 53eb0395a..dc7949e5d 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, prm.Size) + err = t.writer.removeFile(p) 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 50dae46a7..8df61390f 100644 --- a/pkg/local_object_storage/blobstor/fstree/fstree_test.go +++ b/pkg/local_object_storage/blobstor/fstree/fstree_test.go @@ -68,70 +68,34 @@ func TestObjectCounter(t *testing.T) { var delPrm common.DeletePrm delPrm.Address = addr - t.Run("without size hint", func(t *testing.T) { - eg, egCtx := errgroup.WithContext(context.Background()) + 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 - }) - - 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) + } + return nil }) - 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 - } + 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 - }) - - 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) + } + 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) } 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 4110ba7d7..801fc4a22 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, uint64) error + removeFile(string) 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, size uint64) error { +func (w *genericWriter) removeFile(p string) error { var err error if w.fileCounterEnabled { - err = w.removeWithCounter(p, size) + err = w.removeWithCounter(p) } else { err = os.Remove(p) } @@ -121,21 +121,18 @@ func (w *genericWriter) removeFile(p string, size uint64) error { return err } -func (w *genericWriter) removeWithCounter(p string, size uint64) error { +func (w *genericWriter) removeWithCounter(p string) error { w.fileGuard.Lock(p) defer w.fileGuard.Unlock(p) - if size == 0 { - stat, err := os.Stat(p) - if err != nil { - return err - } - size = uint64(stat.Size()) + stat, err := os.Stat(p) + if err != nil { + return err } if err := os.Remove(p); err != nil { return err } - w.fileCounter.Dec(uint64(size)) + w.fileCounter.Dec(uint64(stat.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 3561c616b..3127579ac 100644 --- a/pkg/local_object_storage/blobstor/fstree/fstree_write_linux.go +++ b/pkg/local_object_storage/blobstor/fstree/fstree_write_linux.go @@ -91,30 +91,25 @@ func (w *linuxWriter) writeFile(p string, data []byte) error { return errClose } -func (w *linuxWriter) removeFile(p string, size uint64) error { +func (w *linuxWriter) removeFile(p string) error { if w.fileCounterEnabled { w.fileGuard.Lock(p) defer w.fileGuard.Unlock(p) - - 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) - } } - - err := unix.Unlink(p) + 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 + } + err = unix.Unlink(p) if err != nil && err == unix.ENOENT { return logicerr.Wrap(new(apistatus.ObjectNotFound)) } if err == nil { - w.fileCounter.Dec(uint64(size)) + w.fileCounter.Dec(uint64(stat.Size)) } return err } diff --git a/pkg/local_object_storage/writecache/flush.go b/pkg/local_object_storage/writecache/flush.go index d9e34ceab..6257ed5dd 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, uint64(len(res.RawData))) + c.deleteFromDisk(ctx, objInfo.addr) } func (c *cache) reportFlushError(ctx context.Context, 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, uint64(len(e.ObjectData))) + c.deleteFromDisk(ctx, e.Address) return nil } diff --git a/pkg/local_object_storage/writecache/storage.go b/pkg/local_object_storage/writecache/storage.go index a0e236cb7..b4cc8050f 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, size uint64) { - _, err := c.fsTree.Delete(ctx, common.DeletePrm{Address: addr, Size: size}) +func (c *cache) deleteFromDisk(ctx context.Context, addr oid.Address) { + _, err := c.fsTree.Delete(ctx, common.DeletePrm{Address: addr}) if err != nil && !client.IsErrObjectNotFound(err) { c.log.Error(ctx, logs.WritecacheCantRemoveObjectFromWritecache, zap.Error(err)) } else if err == nil {