Skip to content

Commit

Permalink
scheduler: seed random shuffle nodes with eval ID
Browse files Browse the repository at this point in the history
Processing an evaluation is nearly a pure function over the state
snapshot, but we randomly shuffle the nodes. This means that
developers can't take a given state snapshot and pass an evaluation
through it and be guaranteed the same plan results.

But the evaluation ID is already random, so if we use this as the seed
for shuffling the nodes we can greatly reduce the sources of
non-determinism. Unfortunately golang map iteration uses a global
source of randomness and not a goroutine-local one, but arguably
if the scheduler behavior is impacted by this, that's a bug in the
iteration.
  • Loading branch information
tgross committed Feb 4, 2022
1 parent 290bd0d commit 602150c
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 6 deletions.
3 changes: 3 additions & 0 deletions .changelog/12008.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
scheduler: Seed the shuffling of nodes with the evaluation ID to improve debuggability
```
2 changes: 1 addition & 1 deletion scheduler/feasible.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func (iter *StaticIterator) SetNodes(nodes []*structs.Node) {
// is applied in-place
func NewRandomIterator(ctx Context, nodes []*structs.Node) *StaticIterator {
// shuffle with the Fisher-Yates algorithm
shuffleNodes(nodes)
shuffleNodes(ctx.Plan().EvalID, nodes)

// Create a static iterator
return NewStaticIterator(ctx, nodes)
Expand Down
2 changes: 1 addition & 1 deletion scheduler/stack.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ type GenericStack struct {

func (s *GenericStack) SetNodes(baseNodes []*structs.Node) {
// Shuffle base nodes
shuffleNodes(baseNodes)
shuffleNodes(s.ctx.Plan().EvalID, baseNodes)

// Update the set of base nodes
s.source.SetNodes(baseNodes)
Expand Down
14 changes: 11 additions & 3 deletions scheduler/util.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package scheduler

import (
"encoding/binary"
"fmt"
"math/rand"
"reflect"
Expand Down Expand Up @@ -376,11 +377,18 @@ func taintedNodes(state State, allocs []*structs.Allocation) (map[string]*struct
return out, nil
}

// shuffleNodes randomizes the slice order with the Fisher-Yates algorithm
func shuffleNodes(nodes []*structs.Node) {
// shuffleNodes randomizes the slice order with the Fisher-Yates
// algorithm. We seed the random source with the eval ID (which is
// random) to aid in postmortem debuggging of specific evaluations and
// state snapshots.
func shuffleNodes(evalID string, nodes []*structs.Node) {

seed, _ := binary.Varint([]byte(evalID))
r := rand.New(rand.NewSource(seed))

n := len(nodes)
for i := n - 1; i > 0; i-- {
j := rand.Intn(i + 1)
j := r.Intn(i + 1)
nodes[i], nodes[j] = nodes[j], nodes[i]
}
}
Expand Down
2 changes: 1 addition & 1 deletion scheduler/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ func TestShuffleNodes(t *testing.T) {
}
orig := make([]*structs.Node, len(nodes))
copy(orig, nodes)
shuffleNodes(nodes)
shuffleNodes(uuid.Generate(), nodes)
require.False(t, reflect.DeepEqual(nodes, orig))
}

Expand Down

0 comments on commit 602150c

Please sign in to comment.