From 5b48a48a79341e9997afd47c90df6ab53a0b582f Mon Sep 17 00:00:00 2001 From: Natalie Villasana Date: Tue, 26 Mar 2019 14:09:44 -0400 Subject: [PATCH] adds test for correct download with non-critical amount of nodes offline (#1574) --- internal/testplanet/uplink_test.go | 72 +++++++++++++++++++++++++++ pkg/overlay/cache.go | 2 +- pkg/overlay/cache_test.go | 4 +- satellite/metainfo/metainfo.go | 12 ++++- satellite/satellitedb/overlaycache.go | 2 +- 5 files changed, 86 insertions(+), 6 deletions(-) diff --git a/internal/testplanet/uplink_test.go b/internal/testplanet/uplink_test.go index 94fdcd02d..1125ec784 100644 --- a/internal/testplanet/uplink_test.go +++ b/internal/testplanet/uplink_test.go @@ -13,6 +13,9 @@ import ( "storj.io/storj/internal/memory" "storj.io/storj/internal/testcontext" "storj.io/storj/internal/testplanet" + "storj.io/storj/pkg/pb" + "storj.io/storj/pkg/storj" + "storj.io/storj/uplink" ) func TestUploadDownload(t *testing.T) { @@ -37,3 +40,72 @@ func TestUploadDownload(t *testing.T) { assert.Equal(t, expectedData, data) } + +func TestDownloadWithSomeNodesOffline(t *testing.T) { + testplanet.Run(t, testplanet.Config{ + SatelliteCount: 1, StorageNodeCount: 5, UplinkCount: 1, + }, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet) { + + // first, upload some remote data + ul := planet.Uplinks[0] + satellite := planet.Satellites[0] + + testData := make([]byte, 1*memory.MiB) + _, err := rand.Read(testData) + require.NoError(t, err) + + err = ul.UploadWithConfig(ctx, satellite, &uplink.RSConfig{ + MinThreshold: 2, + RepairThreshold: 3, + SuccessThreshold: 4, + MaxThreshold: 5, + }, "testbucket", "test/path", testData) + require.NoError(t, err) + + // get a remote segment from pointerdb + pdb := satellite.Metainfo.Service + listResponse, _, err := pdb.List("", "", "", true, 0, 0) + require.NoError(t, err) + + var path string + var pointer *pb.Pointer + for _, v := range listResponse { + path = v.GetPath() + pointer, err = pdb.Get(path) + require.NoError(t, err) + if pointer.GetType() == pb.Pointer_REMOTE { + break + } + } + + // calculate how many storagenodes to kill + redundancy := pointer.GetRemote().GetRedundancy() + remotePieces := pointer.GetRemote().GetRemotePieces() + minReq := redundancy.GetMinReq() + numPieces := len(remotePieces) + toKill := numPieces - int(minReq) + + nodesToKill := make(map[storj.NodeID]bool) + for i, piece := range remotePieces { + if i >= toKill { + continue + } + nodesToKill[piece.NodeId] = true + } + + for _, node := range planet.StorageNodes { + if nodesToKill[node.ID()] { + err = planet.StopPeer(node) + require.NoError(t, err) + + err = satellite.Overlay.Service.Delete(ctx, node.ID()) + require.NoError(t, err) + } + } + + // we should be able to download data without any of the original nodes + newData, err := ul.Download(ctx, satellite, "testbucket", "test/path") + require.NoError(t, err) + require.Equal(t, testData, newData) + }) +} diff --git a/pkg/overlay/cache.go b/pkg/overlay/cache.go index 4878aa3e0..0c78400ee 100644 --- a/pkg/overlay/cache.go +++ b/pkg/overlay/cache.go @@ -24,7 +24,7 @@ const ( var ErrEmptyNode = errs.New("empty node ID") // ErrNodeNotFound is returned if a node does not exist in database -var ErrNodeNotFound = errs.New("Node not found") +var ErrNodeNotFound = errs.Class("Node not found") // ErrBucketNotFound is returned if a bucket is unable to be found in the routing table var ErrBucketNotFound = errs.New("Bucket not found") diff --git a/pkg/overlay/cache_test.go b/pkg/overlay/cache_test.go index 919586a26..535542e6f 100644 --- a/pkg/overlay/cache_test.go +++ b/pkg/overlay/cache_test.go @@ -71,7 +71,7 @@ func testCache(ctx context.Context, t *testing.T, store overlay.DB) { invalid2, err := cache.Get(ctx, missingID) assert.Error(t, err) - assert.True(t, err == overlay.ErrNodeNotFound) + assert.True(t, overlay.ErrNodeNotFound.Has(err)) assert.Nil(t, invalid2) // TODO: add erroring database test @@ -130,7 +130,7 @@ func testCache(ctx context.Context, t *testing.T, store overlay.DB) { deleted, err := cache.Get(ctx, valid1ID) assert.Error(t, err) assert.Nil(t, deleted) - assert.True(t, err == overlay.ErrNodeNotFound) + assert.True(t, overlay.ErrNodeNotFound.Has(err)) // Test idempotent delete / non existent key delete err = cache.Delete(ctx, valid1ID) diff --git a/satellite/metainfo/metainfo.go b/satellite/metainfo/metainfo.go index 33b15304e..87edaf53e 100644 --- a/satellite/metainfo/metainfo.go +++ b/satellite/metainfo/metainfo.go @@ -315,6 +315,7 @@ func (endpoint *Endpoint) createOrderLimitsForSegment(ctx context.Context, point pieceSize := eestream.CalcPieceSize(pointer.GetSegmentSize(), redundancy) expiration := pointer.ExpirationDate + var combinedErrs error var limits []*pb.AddressedOrderLimit for _, piece := range pointer.GetRemote().GetRemotePieces() { derivedPieceID := rootPieceID.Derive(piece.NodeId) @@ -325,7 +326,9 @@ func (endpoint *Endpoint) createOrderLimitsForSegment(ctx context.Context, point node, err := endpoint.cache.Get(ctx, piece.NodeId) if err != nil { - return nil, err + endpoint.log.Error("error getting node from overlay cache", zap.Error(err)) + combinedErrs = errs.Combine(combinedErrs, err) + continue } if node != nil { @@ -336,8 +339,13 @@ func (endpoint *Endpoint) createOrderLimitsForSegment(ctx context.Context, point Limit: orderLimit, StorageNodeAddress: node.Address, }) - } + + if len(limits) < redundancy.RequiredCount() { + err = Error.New("not enough nodes available: got %d, required %d", len(limits), redundancy.RequiredCount()) + return nil, errs.Combine(combinedErrs, err) + } + return limits, nil } diff --git a/satellite/satellitedb/overlaycache.go b/satellite/satellitedb/overlaycache.go index d7c2e8493..8552a6116 100644 --- a/satellite/satellitedb/overlaycache.go +++ b/satellite/satellitedb/overlaycache.go @@ -116,7 +116,7 @@ func (cache *overlaycache) Get(ctx context.Context, id storj.NodeID) (*pb.Node, dbx.OverlayCacheNode_NodeId(id.Bytes()), ) if err == sql.ErrNoRows { - return nil, overlay.ErrNodeNotFound + return nil, overlay.ErrNodeNotFound.New("couldn't find nodeID: %s", id.String()) } if err != nil { return nil, err