Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove volume file after volume is removed from the database #340

Merged
merged 1 commit into from
Mar 27, 2024
Merged

Remove volume file after volume is removed from the database #340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions host/storage/storage.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
103 changes: 89 additions & 14 deletions host/storage/storage_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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(&sector)

// write the sector
release, err := vm.Write(roots[i], &sector)
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(&sector)

// write the sector
release, err := vm.Write(root, &sector)
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.
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down