traverser: Check for placement vector out of range #1516
2 changed files with 49 additions and 2 deletions
|
@ -114,8 +114,9 @@ func NewTraverser(opts ...Option) (*Traverser, error) {
|
|||
var unsortedVector []netmap.NodeInfo
|
||||
|
||||
var regularVector []netmap.NodeInfo
|
||||
for i := range rem {
|
||||
unsortedVector = append(unsortedVector, ns[i][:rem[i]]...)
|
||||
regularVector = append(regularVector, ns[i][rem[i]:]...)
|
||||
pivot := min(len(ns[i]), rem[i])
|
||||
dstepanov-yadro
commented
Looks like you have forgotten Looks like you have forgotten `regularVector = append(regularVector, ns[i][rem[i]:]...)` here
fyrchik
commented
But the like you wrote will panic! But the like you wrote will panic!
dstepanov-yadro
commented
I meant that the second slice is not updated. I meant that the second slice is not updated.
acid-ant
commented
It should not because we do not have enough nodes for that in the current vector. It should not because we do not have enough nodes for that in the current vector.
fyrchik
commented
Linter complains about the size of this function.
Linter complains about the size of this function.
How about
```
pivot := min(len(ns[i]), rem[i])
unsortedVector = append(unsortedVector, ns[i][:pivot]...)
regularVector = append(regularVector, ns[i][pivot:]...)
```
fyrchik
commented
It is also somewhat more readable and concise IMO It is also somewhat more readable and concise IMO
acid-ant
commented
Nice! Updated. Nice! Updated.
|
||||
unsortedVector = append(unsortedVector, ns[i][:pivot]...)
|
||||
regularVector = append(regularVector, ns[i][pivot:]...)
|
||||
}
|
||||
rem = []int{-1, -1}
|
||||
|
||||
|
|
|
@ -356,6 +356,52 @@ func TestTraverserPriorityMetrics(t *testing.T) {
|
|||
require.Nil(t, next)
|
||||
})
|
||||
|
||||
t.Run("one rep one metric fewer nodes", func(t *testing.T) {
|
||||
selectors := []int{2}
|
||||
replicas := []int{3}
|
||||
|
||||
nodes, cnr := testPlacement(selectors, replicas)
|
||||
|
||||
// Node_0, PK - ip4/0.0.0.0/tcp/0
|
||||
nodes[0][0].SetAttribute("ClusterName", "A")
|
||||
// Node_1, PK - ip4/0.0.0.0/tcp/1
|
||||
nodes[0][1].SetAttribute("ClusterName", "B")
|
||||
|
||||
sdkNode := testNode(5)
|
||||
sdkNode.SetAttribute("ClusterName", "B")
|
||||
|
||||
nodesCopy := copyVectors(nodes)
|
||||
|
||||
m := []Metric{NewAttributeMetric("ClusterName")}
|
||||
|
||||
tr, err := NewTraverser(
|
||||
ForContainer(cnr),
|
||||
UseBuilder(&testBuilder{
|
||||
vectors: nodesCopy,
|
||||
}),
|
||||
WithoutSuccessTracking(),
|
||||
WithPriorityMetrics(m),
|
||||
WithNodeState(&nodeState{
|
||||
node: &sdkNode,
|
||||
}),
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Without priority metric `ClusterName` the order will be:
|
||||
// [ {Node_0 A}, {Node_1 A} ]
|
||||
// With priority metric `ClusterName` and current node in cluster B
|
||||
// the order should be:
|
||||
// [ {Node_1 B}, {Node_0 A} ]
|
||||
next := tr.Next()
|
||||
require.NotNil(t, next)
|
||||
require.Equal(t, 2, len(next))
|
||||
require.Equal(t, "/ip4/0.0.0.0/tcp/1", string(next[0].PublicKey()))
|
||||
require.Equal(t, "/ip4/0.0.0.0/tcp/0", string(next[1].PublicKey()))
|
||||
|
||||
next = tr.Next()
|
||||
require.Nil(t, next)
|
||||
})
|
||||
|
||||
t.Run("two reps two metrics", func(t *testing.T) {
|
||||
selectors := []int{3, 3}
|
||||
replicas := []int{2, 2}
|
||||
|
|
Loading…
Reference in a new issue
Unrelated non-functional change, please, remove.
Other than this LGTM
Reverted.