Skip to content

Commit 02a7d32

Browse files
committed
asim: ensure consistent sequencing of changes applied to a range
Upon investigating, we identified the non-deterministic behavior was due to the sequence of changes applied on the ranges within one tick. Changes within a tick are applied in one go. But the changes are stored in a map, and map iteration order is non-deterministic. Thus, the sequence of events applied and published via gossip could vary and could result in different event states. This patch resolves the issue by changing the storage of changes to be applied from a map to an array, ensuring consistent iteration and sequence of events. Note the shift to an array doesn't pose any issues since all changes need to be applied anyway. With this fix, the test `TestAsimDeterministic` fails only under race. This is less concerning since the simulator should run on a single goroutine without concurrency. Although the cause of the test failure in race conditions in unclear, fixing this is not a priority. Part Of: cockroachdb#105904 Release Note: None
1 parent 37e61e9 commit 02a7d32

File tree

1 file changed

+4
-4
lines changed

1 file changed

+4
-4
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -477,25 +477,25 @@ func (rc *replicaChanger) Push(tick time.Time, change Change) (time.Time, bool)
477477
// Tick updates state changer to apply any changes that have occurred
478478
// between the last tick and this one.
479479
func (rc *replicaChanger) Tick(tick time.Time, state State) {
480-
changeList := make(map[int]*pendingChange)
480+
var changeList []*pendingChange
481481

482482
// NB: Add the smallest unit of time, in order to find all items in
483483
// [smallest, tick].
484484
pivot := &pendingChange{completeAt: tick.Add(time.Nanosecond)}
485485
rc.completeAt.AscendLessThan(pivot, func(i btree.Item) bool {
486486
nextChange, _ := i.(*pendingChange)
487-
changeList[nextChange.ticket] = nextChange
487+
changeList = append(changeList, nextChange)
488488
return true
489489
})
490490

491-
for ticket, nextChange := range changeList {
491+
for _, nextChange := range changeList {
492492
change := rc.pendingTickets[nextChange.ticket]
493493
change.Apply(state)
494494

495495
// Cleanup the pending trackers for this ticket. This allows another
496496
// change to be pushed for Range().
497497
rc.completeAt.Delete(nextChange)
498-
delete(rc.pendingTickets, ticket)
498+
delete(rc.pendingTickets, nextChange.ticket)
499499
delete(rc.pendingRange, change.Range())
500500
}
501501
}

0 commit comments

Comments
 (0)