From 63942ae2be8cc8a57ff1240815434b287eeaca83 Mon Sep 17 00:00:00 2001 From: Anton Nikiforov Date: Wed, 30 Aug 2023 13:43:43 +0300 Subject: [PATCH] [#665] node: Bind length of copies number to number of replicas Allow to use one digit in copies number array for backward compatibility. Signed-off-by: Anton Nikiforov --- .../object_manager/placement/traverser.go | 8 +++++++ .../placement/traverser_test.go | 21 ++++++++++++------- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/pkg/services/object_manager/placement/traverser.go b/pkg/services/object_manager/placement/traverser.go index ad1077047..dc9ab5e7a 100644 --- a/pkg/services/object_manager/placement/traverser.go +++ b/pkg/services/object_manager/placement/traverser.go @@ -58,6 +58,9 @@ var errNilBuilder = errors.New("placement builder is nil") var errNilPolicy = errors.New("placement policy is nil") +var errCopiesNumberLen = errors.New("copies number accepts only one number or array with length " + + "equal to length of replicas") + func defaultCfg() *cfg { return &cfg{ trackCopies: true, @@ -74,6 +77,11 @@ func NewTraverser(opts ...Option) (*Traverser, error) { } } + cnLen := len(cfg.copyNumbers) + if cnLen > 0 && cnLen != 1 && cnLen != cfg.policy.NumberOfReplicas() { + return nil, errCopiesNumberLen + } + if cfg.builder == nil { return nil, fmt.Errorf("%s: %w", invalidOptsMsg, errNilBuilder) } else if !cfg.policySet { diff --git a/pkg/services/object_manager/placement/traverser_test.go b/pkg/services/object_manager/placement/traverser_test.go index e16f33759..9b70efc73 100644 --- a/pkg/services/object_manager/placement/traverser_test.go +++ b/pkg/services/object_manager/placement/traverser_test.go @@ -220,6 +220,7 @@ func TestTraverserRemValues(t *testing.T) { name string copyNumbers []uint32 expectedRem []int + expectedErr error }{ { name: "zero copy numbers", @@ -231,11 +232,6 @@ func TestTraverserRemValues(t *testing.T) { copyNumbers: []uint32{0}, expectedRem: replicas, }, - { - name: "compatible zero copy numbers, len 2", - copyNumbers: []uint32{0, 0}, - expectedRem: replicas, - }, { name: "compatible zero copy numbers, len 3", copyNumbers: []uint32{0, 0, 0}, @@ -253,9 +249,14 @@ func TestTraverserRemValues(t *testing.T) { }, { name: "multiple copy numbers for multiple replicas", - copyNumbers: []uint32{1, 1}, + copyNumbers: []uint32{1, 1, 4}, expectedRem: []int{1, 1, 4}, }, + { + name: "incompatible copies number vector", + copyNumbers: []uint32{1, 1}, + expectedErr: errCopiesNumberLen, + }, } for _, testCase := range testCases { @@ -265,8 +266,12 @@ func TestTraverserRemValues(t *testing.T) { UseBuilder(&testBuilder{vectors: nodesCopy}), WithCopyNumbers(testCase.copyNumbers), ) - require.NoError(t, err) - require.Equal(t, testCase.expectedRem, tr.rem) + if testCase.expectedErr == nil { + require.NoError(t, err, testCase.name) + require.Equal(t, testCase.expectedRem, tr.rem, testCase.name) + } else { + require.Error(t, err, testCase.expectedErr, testCase.name) + } }) } } -- 2.45.2