diff --git a/.changeset/fix_deletehostsector_deleting_a_sector_from_all_hosts_rather_than_the_given_one.md b/.changeset/fix_deletehostsector_deleting_a_sector_from_all_hosts_rather_than_the_given_one.md new file mode 100644 index 000000000..d62e7a10a --- /dev/null +++ b/.changeset/fix_deletehostsector_deleting_a_sector_from_all_hosts_rather_than_the_given_one.md @@ -0,0 +1,5 @@ +--- +default: patch +--- + +# Fix DeleteHostSector deleting a sector from all hosts rather than the given one diff --git a/internal/test/e2e/cluster.go b/internal/test/e2e/cluster.go index 9564e046f..bc4dacda7 100644 --- a/internal/test/e2e/cluster.go +++ b/internal/test/e2e/cluster.go @@ -639,7 +639,7 @@ func addStorageFolderToHost(ctx context.Context, hosts []*Host) error { // the group func announceHosts(hosts []*Host) error { for _, host := range hosts { - settings := defaultHostSettings + settings := host.settings.Settings() settings.NetAddress = host.rhp4Listener.Addr().(*net.TCPAddr).IP.String() if err := host.UpdateSettings(settings); err != nil { return err diff --git a/internal/test/e2e/cluster_test.go b/internal/test/e2e/cluster_test.go index c1ca0245a..cfc84be4d 100644 --- a/internal/test/e2e/cluster_test.go +++ b/internal/test/e2e/cluster_test.go @@ -1721,6 +1721,30 @@ func TestUploadPacking(t *testing.T) { } else if res.Objects[1].Key != "/file2" { t.Fatal("expected file2", res.Objects[1].Key) } + + // sanity check object metadata + assertObjectMetadata := func(key string) { + t.Helper() + obj, err := b.Object(context.Background(), testBucket, key, api.GetObjectOptions{}) + tt.OK(err) + for _, slab := range obj.Slabs { + if len(slab.Shards) != 3 { + t.Fatalf("unexpected number of shards") + } + for _, shard := range slab.Shards { + if len(shard.Contracts) != 1 { + t.Fatalf("unexpected number of hosts in contracts map") + } + for _, fcids := range shard.Contracts { + if len(fcids) != 1 { + t.Fatalf("unexpected number of contracts") + } + } + } + } + } + assertObjectMetadata("/file1") + assertObjectMetadata("/file2") } func TestWallet(t *testing.T) { diff --git a/internal/test/e2e/gouging_test.go b/internal/test/e2e/gouging_test.go index dc52a39f7..7689f4e27 100644 --- a/internal/test/e2e/gouging_test.go +++ b/internal/test/e2e/gouging_test.go @@ -24,9 +24,20 @@ func TestGouging(t *testing.T) { w := cluster.Worker tt := cluster.tt - // add hosts + // add hosts with short price table validity to speed up test + gs, err := b.GougingSettings(context.Background()) + tt.OK(err) + gs.MinPriceTableValidity = api.DurationMS(10 * time.Second) + tt.OK(cluster.bs.UpdateGougingSettings(context.Background(), gs)) + n := int(test.AutopilotConfig.Contracts.Amount) - tt.OKAll(cluster.AddHostsBlocking(n)) + for i := 0; i < n; i++ { + h := cluster.NewHost() + settings := h.settings.Settings() + settings.PriceTableValidity = time.Duration(gs.MinPriceTableValidity) + h.UpdateSettings(settings) + cluster.AddHost(h) + } cluster.WaitForAccounts() // assert all hosts are usable @@ -66,7 +77,7 @@ func TestGouging(t *testing.T) { tt.OK(cluster.hosts[0].UpdateSettings(updated)) // update gouging settings - gs := test.GougingSettings + gs = test.GougingSettings gs.MaxStoragePrice = settings.StoragePrice if err := b.UpdateGougingSettings(context.Background(), gs); err != nil { t.Fatal(err) @@ -74,13 +85,13 @@ func TestGouging(t *testing.T) { // make sure the price table expires so the worker is forced to fetch it // again, this is necessary for the host to be considered price gouging - time.Sleep(defaultHostSettings.PriceTableValidity) + time.Sleep(time.Duration(gs.MinPriceTableValidity)) // assert all but one host are usable h, err = b.UsableHosts(context.Background()) tt.OK(err) if len(h) != n-1 { - t.Fatal("unexpected number of hosts", len(h)) + t.Fatal("unexpected number of hosts", len(h), n-1) } // upload some data - should fail diff --git a/internal/test/e2e/host.go b/internal/test/e2e/host.go index fe673fa46..948f91fa8 100644 --- a/internal/test/e2e/host.go +++ b/internal/test/e2e/host.go @@ -222,6 +222,7 @@ func NewHost(privKey types.PrivateKey, cm *chain.Manager, dir string, network *c settings.WithRHP2Port(uint16(rhp2Listener.Addr().(*net.TCPAddr).Port)), settings.WithRHP3Port(uint16(rhp3Listener.Addr().(*net.TCPAddr).Port)), settings.WithRHP4Port(uint16(rhp4Listener.Addr().(*net.TCPAddr).Port)), + settings.WithInitialSettings(defaultHostSettings), ) if err != nil { return nil, fmt.Errorf("failed to create settings manager: %w", err) diff --git a/stores/metadata_test.go b/stores/metadata_test.go index 90a386667..bd7546941 100644 --- a/stores/metadata_test.go +++ b/stores/metadata_test.go @@ -3428,6 +3428,7 @@ func TestDeleteHostSector(t *testing.T) { if err != nil { t.Fatal(err) } + hk1FCIDs, hk2FCIDs := fcids[:2], fcids[2:] // create a healthy slab with one sector that is uploaded to all contracts. root := types.Hash256{1, 2, 3} @@ -3436,8 +3437,11 @@ func TestDeleteHostSector(t *testing.T) { MinShards: 1, Shards: []object.Sector{ { - Contracts: map[types.PublicKey][]types.FileContractID{hk1: fcids}, - Root: root, + Contracts: map[types.PublicKey][]types.FileContractID{ + hk1: hk1FCIDs, + hk2: hk2FCIDs, + }, + Root: root, }, }, }) @@ -3447,6 +3451,11 @@ func TestDeleteHostSector(t *testing.T) { t.Fatal("expected 4 contract-sector links", n) } + // Make sure 2 hostSector entries exist. + if n := ss.Count("host_sectors"); n != 2 { + t.Fatal("expected 2 host-sector links", n) + } + // Prune the sector from hk1. if n, err := ss.DeleteHostSector(context.Background(), hk1, root); err != nil { t.Fatal(err) @@ -3459,6 +3468,11 @@ func TestDeleteHostSector(t *testing.T) { t.Fatal("expected 2 contract-sector links", n) } + // Make sure 1 hostSector entry exists. + if n := ss.Count("host_sectors"); n != 1 { + t.Fatal("expected 1 host-sector link", n) + } + // Find the slab. It should have an invalid health. var slabID int64 var validUntil int64 diff --git a/stores/sql/main.go b/stores/sql/main.go index 5e624c25c..1e0661ef3 100644 --- a/stores/sql/main.go +++ b/stores/sql/main.go @@ -555,7 +555,14 @@ func DeleteHostSector(ctx context.Context, tx sql.Tx, hk types.PublicKey, root t } // remove sector from host_sectors - _, err = tx.Exec(ctx, "DELETE FROM host_sectors WHERE db_sector_id = ?", sectorID) + _, err = tx.Exec(ctx, ` + DELETE FROM host_sectors + WHERE db_sector_id = ? AND db_host_id IN ( + SELECT h.id + FROM hosts h + WHERE h.public_key = ? + ) + `, sectorID, PublicKey(hk)) if err != nil { return 0, fmt.Errorf("failed to delete host sector: %w", err) } @@ -2479,7 +2486,7 @@ func MarkPackedSlabUploaded(ctx context.Context, tx Tx, slab api.UploadedPackedS defer hostSectorStmt.Close() // insert shards - for i := range slab.Shards { + for i, sector := range slab.Shards { // insert shard res, err := sectorStmt.Exec(ctx, slabID, i+1, slab.Shards[i].Root[:]) if err != nil { @@ -2490,20 +2497,19 @@ func MarkPackedSlabUploaded(ctx context.Context, tx Tx, slab api.UploadedPackedS return "", fmt.Errorf("failed to get sector id: %w", err) } - // insert contract sector links - for _, sector := range slab.Shards { - uc, ok := usedContracts[sector.ContractID] - if !ok { - continue - } + uc, ok := usedContracts[sector.ContractID] + if !ok { + continue + } - if _, err := contractSectorStmt.Exec(ctx, uc.ID, sectorID); err != nil { - return "", fmt.Errorf("failed to insert contract sector: %w", err) - } + // insert contract sector links + if _, err := contractSectorStmt.Exec(ctx, uc.ID, sectorID); err != nil { + return "", fmt.Errorf("failed to insert contract sector: %w", err) + } - if _, err := hostSectorStmt.Exec(ctx, uc.HostID, sectorID); err != nil { - return "", fmt.Errorf("failed to insert host sector: %w", err) - } + // insert host sector link + if _, err := hostSectorStmt.Exec(ctx, uc.HostID, sectorID); err != nil { + return "", fmt.Errorf("failed to insert host sector: %w", err) } } return bufferFileName, nil