Skip to content

Commit cbfd757

Browse files
committed
asim: sort map by key when iterating
While investigating cockroachdb#105904, we found that the bug's cause is due to the simulator's iteration over an unordered map, leading to non-determinism. There are other occurrences in the simulator's code where unordered map iteration takes place. To make the code less error-prone, this patch checks the simulator's code and sorts unordered maps by key when iterating. While it's uncertain whether this change will solve any underlying non-determinism or pinpoint exactly where in the code non-determinism might occur, there is no harm in making the system less error-prone. Part Of:cockroachdb#105904 Release Note: None
1 parent c730a87 commit cbfd757

File tree

6 files changed

+59
-19
lines changed

6 files changed

+59
-19
lines changed

pkg/kv/kvserver/asim/event/delayed_event.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ func (del DelayedEventList) Len() int { return len(del) }
2424

2525
// Less implements sort.Interface.
2626
func (del DelayedEventList) Less(i, j int) bool {
27+
if del[i].At == del[j].At {
28+
return i < j
29+
}
2730
return del[i].At.Before(del[j].At)
2831
}
2932

pkg/kv/kvserver/asim/gossip/exchange.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,12 @@ func (u *fixedDelayExchange) put(tick time.Time, descs ...roachpb.StoreDescripto
4444
// updates returns back exchanged infos, wrapped as store details that have
4545
// completed between the last tick update was called and the tick given.
4646
func (u *fixedDelayExchange) updates(tick time.Time) []*storepool.StoreDetail {
47-
sort.Slice(u.pending, func(i, j int) bool { return u.pending[i].created.Before(u.pending[j].created) })
47+
sort.Slice(u.pending, func(i, j int) bool {
48+
if u.pending[i].created == u.pending[j].created {
49+
return i < j
50+
}
51+
return u.pending[i].created.Before(u.pending[j].created)
52+
})
4853
ready := []*storepool.StoreDetail{}
4954
i := 0
5055
for ; i < len(u.pending) && !tick.Before(u.pending[i].created.Add(u.settings.StateExchangeDelay)); i++ {

pkg/kv/kvserver/asim/state/impl.go

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,13 @@ func (s *state) ClusterInfo() ClusterInfo {
178178
// Stores returns all stores that exist in this state.
179179
func (s *state) Stores() []Store {
180180
stores := make([]Store, 0, len(s.stores))
181-
for _, store := range s.stores {
181+
keys := make([]StoreID, 0, len(s.stores))
182+
for key := range s.stores {
183+
keys = append(keys, key)
184+
}
185+
186+
for _, key := range keys {
187+
store := s.stores[key]
182188
stores = append(stores, store)
183189
}
184190
sort.Slice(stores, func(i, j int) bool { return stores[i].StoreID() < stores[j].StoreID() })
@@ -344,7 +350,12 @@ func (s *state) Replicas(storeID StoreID) []Replica {
344350
}
345351

346352
repls := make(replicaList, 0, len(store.replicas))
353+
var rangeIDs RangeIDSlice
347354
for rangeID := range store.replicas {
355+
rangeIDs = append(rangeIDs, rangeID)
356+
}
357+
sort.Sort(rangeIDs)
358+
for _, rangeID := range rangeIDs {
348359
rng := s.ranges.rangeMap[rangeID]
349360
if replica := rng.replicas[storeID]; replica != nil {
350361
repls = append(repls, replica)
@@ -1017,7 +1028,13 @@ func (s *state) TickClock(tick time.Time) {
10171028
func (s *state) UpdateStorePool(
10181029
storeID StoreID, storeDescriptors map[roachpb.StoreID]*storepool.StoreDetail,
10191030
) {
1020-
for gossipStoreID, detail := range storeDescriptors {
1031+
var storeIDs roachpb.StoreIDSlice
1032+
for storeIDA := range storeDescriptors {
1033+
storeIDs = append(storeIDs, storeIDA)
1034+
}
1035+
sort.Sort(storeIDs)
1036+
for _, gossipStoreID := range storeIDs {
1037+
detail := storeDescriptors[gossipStoreID]
10211038
copiedDetail := *detail
10221039
copiedDesc := *detail.Desc
10231040
copiedDetail.Desc = &copiedDesc

pkg/kv/kvserver/asim/state/new_state.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ type requestCounts []requestCount
2727

2828
func (s requestCounts) Len() int { return len(s) }
2929
func (s requestCounts) Less(i, j int) bool {
30+
if s[i].req == s[j].req {
31+
return i < j
32+
}
3033
return s[i].req > s[j].req
3134
}
3235
func (s requestCounts) Swap(i, j int) { s[i], s[j] = s[j], s[i] }

pkg/kv/kvserver/asim/state/state.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,18 @@ type (
3636
RangeID int32
3737
)
3838

39+
type RangeIDSlice []RangeID
40+
41+
func (r RangeIDSlice) Len() int { return len(r) }
42+
func (r RangeIDSlice) Swap(i, j int) { r[i], r[j] = r[j], r[i] }
43+
func (r RangeIDSlice) Less(i, j int) bool { return r[i] < r[j] }
44+
45+
type StoreIDSlice []StoreID
46+
47+
func (r StoreIDSlice) Len() int { return len(r) }
48+
func (r StoreIDSlice) Swap(i, j int) { r[i], r[j] = r[j], r[i] }
49+
func (r StoreIDSlice) Less(i, j int) bool { return r[i] < r[j] }
50+
3951
// State encapsulates the current configuration and load of a simulation run.
4052
// It provides methods for accessing and mutation simulation state of nodes,
4153
// stores, ranges and replicas.

pkg/kv/kvserver/asim/tests/testdata/example_zone_config

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -55,22 +55,22 @@ plot stat=replicas
5555
----
5656
----
5757

58-
51.00 ┤ ╭╭──────────────────────────────────────────────────────────────
59-
47.60╭╮╭─╭╯─────────────────────────────────────────────────────────────
60-
44.20 │╭──
61-
40.80 ╭╭╯╯
62-
37.40╭─
63-
34.00│─
64-
30.60
65-
27.20╭│
66-
23.80╭╯
67-
20.40
68-
17.00 ┼────────╮╮
69-
13.60╮╮
70-
10.20 ┤ ╰│╰╮╮
71-
6.80 ┤ ╰─╮───╮
72-
3.40 ┤ ╰╰╰───╮──────╮
73-
0.00 ┤ ╰╰╰╰───────────────────────────────────────────────────────────────
58+
52.00 ┤ ╭╭────────────────────────────────────────────────────────────
59+
48.53 ╭╭─────────────────────────────────────────────────────────────────
60+
45.07╭╮││─╯
61+
41.60╭│╭─╯╯
62+
38.13│││
63+
34.67╭╭
64+
31.20╭╭
65+
27.73││╯
66+
24.27││
67+
20.80╭╭
68+
17.33 ┼────────╮╮
69+
13.87╰╰╮╮╮
70+
10.40 ┤ ╰╰─╮╮
71+
6.93 ┤ ╰╰╰──╮
72+
3.47 ┤ ╰╰╰╰─╰────╮
73+
0.00 ┤ ╰╰╰╰───────────────────────────────────────────────────────────────
7474
replicas
7575
----
7676
----

0 commit comments

Comments
 (0)