From 678b859172ad35594afb588e1e06da70e18c28a1 Mon Sep 17 00:00:00 2001 From: Egon Elbre Date: Thu, 7 May 2020 14:54:48 +0300 Subject: [PATCH] satellite/overlay: remove MinimumRequiredNodes In non-test code we were only using RequestedCount, not need to have MinimumRequiredNodes. Change-Id: I40736f4b028b41e94abfdeb221bce5aa86a5cb82 --- satellite/audit/disqualification_test.go | 7 ++-- satellite/overlay/benchmark_test.go | 42 +++++++++----------- satellite/overlay/nodeselectioncache.go | 5 +-- satellite/overlay/nodeselectioncache_test.go | 14 +++---- satellite/overlay/selection_test.go | 18 ++++----- satellite/overlay/service.go | 12 ++---- 6 files changed, 40 insertions(+), 58 deletions(-) diff --git a/satellite/audit/disqualification_test.go b/satellite/audit/disqualification_test.go index 7075ee1d1..05d3718fa 100644 --- a/satellite/audit/disqualification_test.go +++ b/satellite/audit/disqualification_test.go @@ -169,10 +169,9 @@ func TestDisqualifiedNodesGetNoUpload(t *testing.T) { require.NoError(t, err) request := overlay.FindStorageNodesRequest{ - MinimumRequiredNodes: 4, - RequestedCount: 4, - ExcludedIDs: nil, - MinimumVersion: "", // semver or empty + RequestedCount: 4, + ExcludedIDs: nil, + MinimumVersion: "", // semver or empty } nodes, err := satellitePeer.Overlay.Service.FindStorageNodesForUpload(ctx, request) assert.True(t, overlay.ErrNotEnoughNodes.Has(err)) diff --git a/satellite/overlay/benchmark_test.go b/satellite/overlay/benchmark_test.go index 4f2581925..80af91a9b 100644 --- a/satellite/overlay/benchmark_test.go +++ b/satellite/overlay/benchmark_test.go @@ -385,10 +385,9 @@ func BenchmarkNodeSelection(b *testing.B) { b.Run("FindStorageNodesWithPreference", func(b *testing.B) { for i := 0; i < b.N; i++ { selected, err := service.FindStorageNodesWithPreferences(ctx, overlay.FindStorageNodesRequest{ - MinimumRequiredNodes: SelectCount, - RequestedCount: SelectCount, - ExcludedIDs: nil, - MinimumVersion: "v1.0.0", + RequestedCount: SelectCount, + ExcludedIDs: nil, + MinimumVersion: "v1.0.0", }, &nodeSelectionConfig) require.NoError(b, err) require.NotEmpty(b, selected) @@ -398,10 +397,9 @@ func BenchmarkNodeSelection(b *testing.B) { b.Run("FindStorageNodesWithPreferenceExclusion", func(b *testing.B) { for i := 0; i < b.N; i++ { selected, err := service.FindStorageNodesWithPreferences(ctx, overlay.FindStorageNodesRequest{ - MinimumRequiredNodes: SelectCount, - RequestedCount: SelectCount, - ExcludedIDs: excludedIDs, - MinimumVersion: "v1.0.0", + RequestedCount: SelectCount, + ExcludedIDs: excludedIDs, + MinimumVersion: "v1.0.0", }, &nodeSelectionConfig) require.NoError(b, err) require.NotEmpty(b, selected) @@ -411,10 +409,9 @@ func BenchmarkNodeSelection(b *testing.B) { b.Run("FindStorageNodes", func(b *testing.B) { for i := 0; i < b.N; i++ { selected, err := service.FindStorageNodesForUpload(ctx, overlay.FindStorageNodesRequest{ - MinimumRequiredNodes: SelectCount, - RequestedCount: SelectCount, - ExcludedIDs: nil, - MinimumVersion: "v1.0.0", + RequestedCount: SelectCount, + ExcludedIDs: nil, + MinimumVersion: "v1.0.0", }) require.NoError(b, err) require.NotEmpty(b, selected) @@ -424,10 +421,9 @@ func BenchmarkNodeSelection(b *testing.B) { b.Run("FindStorageNodesExclusion", func(b *testing.B) { for i := 0; i < b.N; i++ { selected, err := service.FindStorageNodesForUpload(ctx, overlay.FindStorageNodesRequest{ - MinimumRequiredNodes: SelectCount, - RequestedCount: SelectCount, - ExcludedIDs: excludedIDs, - MinimumVersion: "v1.0.0", + RequestedCount: SelectCount, + ExcludedIDs: excludedIDs, + MinimumVersion: "v1.0.0", }) require.NoError(b, err) require.NotEmpty(b, selected) @@ -437,10 +433,9 @@ func BenchmarkNodeSelection(b *testing.B) { b.Run("NodeSelectionCacheGetNodes", func(b *testing.B) { for i := 0; i < b.N; i++ { selected, err := service.SelectionCache.GetNodes(ctx, overlay.FindStorageNodesRequest{ - MinimumRequiredNodes: SelectCount, - RequestedCount: SelectCount, - ExcludedIDs: nil, - MinimumVersion: "v1.0.0", + RequestedCount: SelectCount, + ExcludedIDs: nil, + MinimumVersion: "v1.0.0", }) require.NoError(b, err) require.NotEmpty(b, selected) @@ -450,10 +445,9 @@ func BenchmarkNodeSelection(b *testing.B) { b.Run("NodeSelectionCacheGetNodesExclusion", func(b *testing.B) { for i := 0; i < b.N; i++ { selected, err := service.SelectionCache.GetNodes(ctx, overlay.FindStorageNodesRequest{ - MinimumRequiredNodes: SelectCount, - RequestedCount: SelectCount, - ExcludedIDs: excludedIDs, - MinimumVersion: "v1.0.0", + RequestedCount: SelectCount, + ExcludedIDs: excludedIDs, + MinimumVersion: "v1.0.0", }) require.NoError(b, err) require.NotEmpty(b, selected) diff --git a/satellite/overlay/nodeselectioncache.go b/satellite/overlay/nodeselectioncache.go index 492162a1d..608cf96bb 100644 --- a/satellite/overlay/nodeselectioncache.go +++ b/satellite/overlay/nodeselectioncache.go @@ -127,10 +127,7 @@ func (cacheData *state) GetNodes(ctx context.Context, req FindStorageNodesReques defer cacheData.mu.RUnlock() // how many reputableNodes versus newNode nodes should be selected - totalcount := req.MinimumRequiredNodes - if totalcount <= 0 { - totalcount = req.RequestedCount - } + totalcount := req.RequestedCount newNodeCount := int(float64(totalcount) * newNodeFraction) var selectedNodeResults = []*SelectedNode{} diff --git a/satellite/overlay/nodeselectioncache_test.go b/satellite/overlay/nodeselectioncache_test.go index ff2ae0708..9606fbf91 100644 --- a/satellite/overlay/nodeselectioncache_test.go +++ b/satellite/overlay/nodeselectioncache_test.go @@ -261,7 +261,7 @@ func TestGetNodesConcurrent(t *testing.T) { var group errgroup.Group group.Go(func() error { nodes, err := cache.GetNodes(ctx, overlay.FindStorageNodesRequest{ - MinimumRequiredNodes: 1, + RequestedCount: 1, }) for i := range nodes { nodes[i].ID = storj.NodeID{byte(i)} @@ -272,7 +272,7 @@ func TestGetNodesConcurrent(t *testing.T) { }) group.Go(func() error { nodes, err := cache.GetNodes(ctx, overlay.FindStorageNodesRequest{ - MinimumRequiredNodes: 1, + RequestedCount: 1, }) for i := range nodes { nodes[i].ID = storj.NodeID{byte(i)} @@ -300,7 +300,7 @@ func TestGetNodesConcurrent(t *testing.T) { group.Go(func() error { nodes, err := cache.GetNodes(ctx, overlay.FindStorageNodesRequest{ - MinimumRequiredNodes: 1, + RequestedCount: 1, }) for i := range nodes { nodes[i].ID = storj.NodeID{byte(i)} @@ -311,7 +311,7 @@ func TestGetNodesConcurrent(t *testing.T) { }) group.Go(func() error { nodes, err := cache.GetNodes(ctx, overlay.FindStorageNodesRequest{ - MinimumRequiredNodes: 1, + RequestedCount: 1, }) for i := range nodes { nodes[i].ID = storj.NodeID{byte(i)} @@ -387,7 +387,7 @@ func TestGetNodesDistinct(t *testing.T) { // selecting 3 should be possible nodes, err := cache.GetNodes(ctx, overlay.FindStorageNodesRequest{ - MinimumRequiredNodes: 3, + RequestedCount: 3, }) require.NoError(t, err) seen := make(map[string]bool) @@ -398,7 +398,7 @@ func TestGetNodesDistinct(t *testing.T) { // selecting 6 is impossible _, err = cache.GetNodes(ctx, overlay.FindStorageNodesRequest{ - MinimumRequiredNodes: 6, + RequestedCount: 6, }) require.Error(t, err) } @@ -414,7 +414,7 @@ func TestGetNodesDistinct(t *testing.T) { ) _, err := cache.GetNodes(ctx, overlay.FindStorageNodesRequest{ - MinimumRequiredNodes: 6, + RequestedCount: 6, }) require.NoError(t, err) } diff --git a/satellite/overlay/selection_test.go b/satellite/overlay/selection_test.go index 22d53c8d2..228a8f7b1 100644 --- a/satellite/overlay/selection_test.go +++ b/satellite/overlay/selection_test.go @@ -71,8 +71,7 @@ func TestMinimumDiskSpace(t *testing.T) { require.NoError(t, err) req := overlay.FindStorageNodesRequest{ - MinimumRequiredNodes: 2, - RequestedCount: 2, + RequestedCount: 2, } // request 2 nodes, expect failure from not enough nodes @@ -541,9 +540,8 @@ func TestFindStorageNodesDistinctNetworks(t *testing.T) { excludedNodeAddr = res.LastIPPort req := overlay.FindStorageNodesRequest{ - MinimumRequiredNodes: 2, - RequestedCount: 2, - ExcludedIDs: excludedNodes, + RequestedCount: 2, + ExcludedIDs: excludedNodes, } nodes, err := satellite.Overlay.Service.FindStorageNodesForUpload(ctx, req) require.NoError(t, err) @@ -565,9 +563,8 @@ func TestFindStorageNodesDistinctNetworks(t *testing.T) { require.NotEqual(t, n3[1].LastIPPort, excludedNodeAddr) req = overlay.FindStorageNodesRequest{ - MinimumRequiredNodes: 4, - RequestedCount: 4, - ExcludedIDs: excludedNodes, + RequestedCount: 4, + ExcludedIDs: excludedNodes, } n, err := satellite.Overlay.Service.FindStorageNodesForUpload(ctx, req) require.Error(t, err) @@ -620,9 +617,8 @@ func TestSelectNewStorageNodesExcludedIPs(t *testing.T) { excludedNodeAddr = res.LastIPPort req := overlay.FindStorageNodesRequest{ - MinimumRequiredNodes: 2, - RequestedCount: 2, - ExcludedIDs: excludedNodes, + RequestedCount: 2, + ExcludedIDs: excludedNodes, } nodes, err := satellite.Overlay.Service.FindStorageNodesForUpload(ctx, req) require.NoError(t, err) diff --git a/satellite/overlay/service.go b/satellite/overlay/service.go index 19927edcd..0ef1e1c4a 100644 --- a/satellite/overlay/service.go +++ b/satellite/overlay/service.go @@ -111,10 +111,9 @@ type NodeCheckInInfo struct { // FindStorageNodesRequest defines easy request parameters. type FindStorageNodesRequest struct { - MinimumRequiredNodes int - RequestedCount int - ExcludedIDs []storj.NodeID - MinimumVersion string // semver or empty + RequestedCount int + ExcludedIDs []storj.NodeID + MinimumVersion string // semver or empty } // NodeCriteria are the requirements for selecting nodes @@ -338,10 +337,7 @@ func (service *Service) FindStorageNodesWithPreferences(ctx context.Context, req // TODO: add sanity limits to requested node count // TODO: add sanity limits to excluded nodes - totalNeededNodes := req.MinimumRequiredNodes - if totalNeededNodes <= 0 { - totalNeededNodes = req.RequestedCount - } + totalNeededNodes := req.RequestedCount excludedIDs := req.ExcludedIDs // if distinctIP is enabled, keep track of the network