Skip to content

Commit 7bd7c00

Browse files
jakobhtins-tril
authored andcommitted
Determinized the shard assignments (cadence-workflow#7184)
What changed? We now sort the executors before we process the shard assignement. Why? We don't want to rely on map ordering since it's random in Go. This made tests flaky, and also would make debugging harder. How did you test it? Unit tests Potential risks Release notes Documentation Changes
1 parent a6addbe commit 7bd7c00

File tree

2 files changed

+8
-12
lines changed

2 files changed

+8
-12
lines changed

service/sharddistributor/leader/process/processor.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"math/rand"
7+
"slices"
78
"sort"
89
"strconv"
910
"sync"
@@ -401,7 +402,14 @@ func assignShardsToEmptyExecutors(currentAssignments map[string][]string) bool {
401402
executorsWithShards := make([]string, 0)
402403
minShardsCurrentlyAssigned := 0
403404

405+
// Ensure the iteration is deterministic.
406+
executors := make([]string, 0, len(currentAssignments))
404407
for executorID := range currentAssignments {
408+
executors = append(executors, executorID)
409+
}
410+
slices.Sort(executors)
411+
412+
for _, executorID := range executors {
405413
if len(currentAssignments[executorID]) == 0 {
406414
emptyExecutors = append(emptyExecutors, executorID)
407415
} else {

service/sharddistributor/leader/process/processor_test.go

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package process
33
import (
44
"context"
55
"errors"
6-
"slices"
76
"sync"
87
"testing"
98
"time"
@@ -399,7 +398,6 @@ func TestGetShards_Utility(t *testing.T) {
399398
}
400399

401400
func TestAssignShardsToEmptyExecutors(t *testing.T) {
402-
t.Skip("Skipping this test for now because it's flaky")
403401
cases := []struct {
404402
name string
405403
inputAssignments map[string][]string
@@ -468,18 +466,8 @@ func TestAssignShardsToEmptyExecutors(t *testing.T) {
468466
t.Run(c.name, func(t *testing.T) {
469467
actualDistributionChanged := assignShardsToEmptyExecutors(c.inputAssignments)
470468

471-
// Sort the assignments, so the test is stable
472-
sortAssignments(c.expectedAssignments)
473-
sortAssignments(c.inputAssignments)
474-
475469
assert.Equal(t, c.expectedAssignments, c.inputAssignments)
476470
assert.Equal(t, c.expectedDistributonChanged, actualDistributionChanged)
477471
})
478472
}
479473
}
480-
481-
func sortAssignments(assignments map[string][]string) {
482-
for _, assignment := range assignments {
483-
slices.Sort(assignment)
484-
}
485-
}

0 commit comments

Comments
 (0)