Skip to content

Commit

Permalink
asim: sort map by key when iterating
Browse files Browse the repository at this point in the history
While investigating #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:#105904
Release Note: None
  • Loading branch information
wenyihu6 committed Aug 1, 2023
1 parent c730a87 commit cbfd757
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 19 deletions.
3 changes: 3 additions & 0 deletions pkg/kv/kvserver/asim/event/delayed_event.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ func (del DelayedEventList) Len() int { return len(del) }

// Less implements sort.Interface.
func (del DelayedEventList) Less(i, j int) bool {
if del[i].At == del[j].At {
return i < j
}
return del[i].At.Before(del[j].At)
}

Expand Down
7 changes: 6 additions & 1 deletion pkg/kv/kvserver/asim/gossip/exchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,12 @@ func (u *fixedDelayExchange) put(tick time.Time, descs ...roachpb.StoreDescripto
// updates returns back exchanged infos, wrapped as store details that have
// completed between the last tick update was called and the tick given.
func (u *fixedDelayExchange) updates(tick time.Time) []*storepool.StoreDetail {
sort.Slice(u.pending, func(i, j int) bool { return u.pending[i].created.Before(u.pending[j].created) })
sort.Slice(u.pending, func(i, j int) bool {
if u.pending[i].created == u.pending[j].created {
return i < j
}
return u.pending[i].created.Before(u.pending[j].created)
})
ready := []*storepool.StoreDetail{}
i := 0
for ; i < len(u.pending) && !tick.Before(u.pending[i].created.Add(u.settings.StateExchangeDelay)); i++ {
Expand Down
21 changes: 19 additions & 2 deletions pkg/kv/kvserver/asim/state/impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,13 @@ func (s *state) ClusterInfo() ClusterInfo {
// Stores returns all stores that exist in this state.
func (s *state) Stores() []Store {
stores := make([]Store, 0, len(s.stores))
for _, store := range s.stores {
keys := make([]StoreID, 0, len(s.stores))
for key := range s.stores {
keys = append(keys, key)
}

for _, key := range keys {
store := s.stores[key]
stores = append(stores, store)
}
sort.Slice(stores, func(i, j int) bool { return stores[i].StoreID() < stores[j].StoreID() })
Expand Down Expand Up @@ -344,7 +350,12 @@ func (s *state) Replicas(storeID StoreID) []Replica {
}

repls := make(replicaList, 0, len(store.replicas))
var rangeIDs RangeIDSlice
for rangeID := range store.replicas {
rangeIDs = append(rangeIDs, rangeID)
}
sort.Sort(rangeIDs)
for _, rangeID := range rangeIDs {
rng := s.ranges.rangeMap[rangeID]
if replica := rng.replicas[storeID]; replica != nil {
repls = append(repls, replica)
Expand Down Expand Up @@ -1017,7 +1028,13 @@ func (s *state) TickClock(tick time.Time) {
func (s *state) UpdateStorePool(
storeID StoreID, storeDescriptors map[roachpb.StoreID]*storepool.StoreDetail,
) {
for gossipStoreID, detail := range storeDescriptors {
var storeIDs roachpb.StoreIDSlice
for storeIDA := range storeDescriptors {
storeIDs = append(storeIDs, storeIDA)
}
sort.Sort(storeIDs)
for _, gossipStoreID := range storeIDs {
detail := storeDescriptors[gossipStoreID]
copiedDetail := *detail
copiedDesc := *detail.Desc
copiedDetail.Desc = &copiedDesc
Expand Down
3 changes: 3 additions & 0 deletions pkg/kv/kvserver/asim/state/new_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ type requestCounts []requestCount

func (s requestCounts) Len() int { return len(s) }
func (s requestCounts) Less(i, j int) bool {
if s[i].req == s[j].req {
return i < j
}
return s[i].req > s[j].req
}
func (s requestCounts) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
Expand Down
12 changes: 12 additions & 0 deletions pkg/kv/kvserver/asim/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,18 @@ type (
RangeID int32
)

type RangeIDSlice []RangeID

func (r RangeIDSlice) Len() int { return len(r) }
func (r RangeIDSlice) Swap(i, j int) { r[i], r[j] = r[j], r[i] }
func (r RangeIDSlice) Less(i, j int) bool { return r[i] < r[j] }

type StoreIDSlice []StoreID

func (r StoreIDSlice) Len() int { return len(r) }
func (r StoreIDSlice) Swap(i, j int) { r[i], r[j] = r[j], r[i] }
func (r StoreIDSlice) Less(i, j int) bool { return r[i] < r[j] }

// State encapsulates the current configuration and load of a simulation run.
// It provides methods for accessing and mutation simulation state of nodes,
// stores, ranges and replicas.
Expand Down
32 changes: 16 additions & 16 deletions pkg/kv/kvserver/asim/tests/testdata/example_zone_config
Original file line number Diff line number Diff line change
Expand Up @@ -55,22 +55,22 @@ plot stat=replicas
----
----

51.00 ┤ ╭╭──────────────────────────────────────────────────────────────
47.60╭╮╭─╭╯─────────────────────────────────────────────────────────────
44.20 │╭──
40.80 ╭╭╯╯
37.40╭─
34.00│─
30.60
27.20╭│
23.80╭╯
20.40
17.00 ┼────────╮╮
13.60╮╮
10.20 ┤ ╰│╰╮╮
6.80 ┤ ╰─╮───╮
3.40 ┤ ╰╰╰───╮──────╮
0.00 ┤ ╰╰╰╰───────────────────────────────────────────────────────────────
52.00 ┤ ╭╭────────────────────────────────────────────────────────────
48.53 ╭╭─────────────────────────────────────────────────────────────────
45.07╭╮││─╯
41.60╭│╭─╯╯
38.13│││
34.67╭╭
31.20╭╭
27.73││╯
24.27││
20.80╭╭
17.33 ┼────────╮╮
13.87╰╰╮╮╮
10.40 ┤ ╰╰─╮╮
6.93 ┤ ╰╰╰──╮
3.47 ┤ ╰╰╰╰─╰────╮
0.00 ┤ ╰╰╰╰───────────────────────────────────────────────────────────────
replicas
----
----

0 comments on commit cbfd757

Please sign in to comment.