From 58fb644c08089e1355306dc3255c057a8d009964 Mon Sep 17 00:00:00 2001 From: Nate Maninger Date: Sat, 16 Mar 2024 11:20:12 -0700 Subject: [PATCH] storage: remove volume file after volume is removed from the database --- host/storage/storage.go | 26 ++++----- host/storage/storage_test.go | 103 ++++++++++++++++++++++++++++++----- 2 files changed, 102 insertions(+), 27 deletions(-) diff --git a/host/storage/storage.go b/host/storage/storage.go index 76f25ece..94bf8abf 100644 --- a/host/storage/storage.go +++ b/host/storage/storage.go @@ -602,25 +602,25 @@ func (vm *VolumeManager) RemoveVolume(ctx context.Context, id int64, force bool, return } - // close the volume and remove it from memory - if err := vol.Close(); err != nil { - log.Error("failed to close volume", zap.Error(err)) - updateRemovalAlert("Failed to remove volume", alerts.SeverityError, err) - result <- err - return - } else if err := os.Remove(stat.LocalPath); err != nil && !errors.Is(err, os.ErrNotExist) { - log.Error("failed to remove volume file", zap.Error(err)) + // remove the volume from the volume store + if err := vm.vs.RemoveVolume(id, force); err != nil { + log.Error("failed to remove volume", zap.Error(err)) + // update the alert updateRemovalAlert("Failed to remove volume", alerts.SeverityError, err) result <- err return } delete(vm.volumes, id) - // remove the volume from the volume store - if err := vm.vs.RemoveVolume(id, force); err != nil { - log.Error("failed to remove volume", zap.Error(err)) - // update the alert - updateRemovalAlert("Failed to remove volume", alerts.SeverityError, err) + // close the volume file and remove it from disk + if err := vol.Close(); err != nil { + log.Error("failed to close volume", zap.Error(err)) + updateRemovalAlert("Failed to close volume files", alerts.SeverityError, err) + result <- err + return + } else if err := os.Remove(stat.LocalPath); err != nil && !errors.Is(err, os.ErrNotExist) { + log.Error("failed to remove volume file", zap.Error(err)) + updateRemovalAlert("Failed to delete volume file", alerts.SeverityError, err) result <- err return } diff --git a/host/storage/storage_test.go b/host/storage/storage_test.go index 8a4a6ec8..1d83ddb7 100644 --- a/host/storage/storage_test.go +++ b/host/storage/storage_test.go @@ -257,7 +257,7 @@ func TestRemoveVolume(t *testing.T) { } am := alerts.NewManager(webhookReporter, log.Named("alerts")) - vm, err := storage.NewVolumeManager(db, am, cm, log.Named("volumes"), sectorCacheSize) + vm, err := storage.NewVolumeManager(db, am, cm, log.Named("volumes"), 0) if err != nil { t.Fatal(err) } @@ -272,18 +272,48 @@ func TestRemoveVolume(t *testing.T) { t.Fatal(err) } - var sector [rhp2.SectorSize]byte - if _, err := frand.Read(sector[:256]); err != nil { - t.Fatal(err) + roots := make([]types.Hash256, 10) + for i := range roots { + var sector [rhp2.SectorSize]byte + if _, err := frand.Read(sector[:256]); err != nil { + t.Fatal(err) + } + roots[i] = rhp2.SectorRoot(§or) + + // write the sector + release, err := vm.Write(roots[i], §or) + if err != nil { + t.Fatal(err) + } else if err := vm.AddTemporarySectors([]storage.TempSector{{Root: roots[i], Expiration: 1}}); err != nil { // must add a temp sector to prevent pruning + t.Fatal(err) + } else if err := release(); err != nil { + t.Fatal(err) + } } - root := rhp2.SectorRoot(§or) - // write the sector - release, err := vm.Write(root, §or) - if err != nil { - t.Fatal(err) + checkRoots := func(roots []types.Hash256) error { + for _, root := range roots { + sector, err := vm.Read(root) + if err != nil { + return fmt.Errorf("failed to read sector: %w", err) + } else if rhp2.SectorRoot(sector) != root { + return errors.New("sector was corrupted") + } + } + return nil + } + + checkVolume := func(id int64, used, total uint64) error { + vol, err := vm.Volume(id) + if err != nil { + return fmt.Errorf("failed to get volume: %w", err) + } else if vol.UsedSectors != used { + return fmt.Errorf("expected %v used sectors, got %v", used, vol.UsedSectors) + } else if vol.TotalSectors != total { + return fmt.Errorf("expected %v total sectors, got %v", total, vol.TotalSectors) + } + return nil } - defer release() // attempt to remove the volume. Should return ErrMigrationFailed since // there is only one volume. @@ -295,12 +325,57 @@ func TestRemoveVolume(t *testing.T) { t.Fatalf("expected ErrNotEnoughStorage, got %v", err) } - // remove the sector - if err := vm.RemoveSector(root); err != nil { + // check that the volume metrics did not change + if err := checkRoots(roots); err != nil { + t.Fatal(err) + } else if err := checkVolume(volume.ID, 10, expectedSectors); err != nil { t.Fatal(err) } - // remove the volume + // add a second volume with enough space to migrate half the sectors + volume2, err := vm.AddVolume(context.Background(), filepath.Join(t.TempDir(), "hostdata2.dat"), uint64(len(roots)/2), result) + if err != nil { + t.Fatal(err) + } else if err := <-result; err != nil { + t.Fatal(err) + } + + if err := checkVolume(volume2.ID, 0, uint64(len(roots)/2)); err != nil { + t.Fatal(err) + } + + // remove the volume first volume. Should still fail with ErrMigrationFailed, + // but some sectors should be migrated to the second volume. + if err := vm.RemoveVolume(context.Background(), volume.ID, false, result); err != nil { + t.Fatal(err) + } else if err := <-result; !errors.Is(err, storage.ErrMigrationFailed) { + t.Fatal(err) + } + + if err := checkRoots(roots); err != nil { + t.Fatal(err) + } else if err := checkVolume(volume.ID, 5, expectedSectors); err != nil { // half the sectors should have been migrated + t.Fatal(err) + } else if err := checkVolume(volume2.ID, 5, uint64(len(roots)/2)); err != nil { + t.Fatal(err) + } + + // expand the second volume to accept the remaining sectors + if err := vm.ResizeVolume(context.Background(), volume2.ID, expectedSectors, result); err != nil { + t.Fatal(err) + } else if err := <-result; err != nil { + t.Fatal(err) + } + + if err := checkVolume(volume2.ID, 5, expectedSectors); err != nil { + t.Fatal(err) + } else if err := checkVolume(volume.ID, 5, expectedSectors); err != nil { + t.Fatal(err) + } else if err := checkRoots(roots); err != nil { + t.Fatal(err) + } + + // remove the first volume if err := vm.RemoveVolume(context.Background(), volume.ID, false, result); err != nil { t.Fatal(err) } else if err := <-result; err != nil { @@ -550,7 +625,7 @@ func TestRemoveMissing(t *testing.T) { } am := alerts.NewManager(webhookReporter, log.Named("alerts")) - vm, err := storage.NewVolumeManager(db, am, cm, log.Named("volumes"), sectorCacheSize) + vm, err := storage.NewVolumeManager(db, am, cm, log.Named("volumes"), 0) if err != nil { t.Fatal(err) }