Merge cockroachdb#107696 cockroachdb#107840 cockroachdb#107870 cockroachdb#107964 cockroachdb#107979 cockroachdb#107981

craig[bot] · wenyihu6 · sjbarag · craig[bot] · commit 6eb06ac72800 · 2023-08-02T14:57:47.000Z
107696: asim: ensure consistent sequencing of changes applied to ranges r=kvoli a=wenyihu6 **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 ---- **asim: add tests for constraints validation, asim determinism** This patch adds an asim datadriven test case which sets up zone constraints and ensures its proper application and validation of these constraints. Part Of: cockroachdb#105904 Release Note: None ---- **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 ---- Each test were stressed with [runs](https://github.com/cockroachdb/cockroach/blob/cb4f59df66852e24546067d4904a4c981b0cc92e/pkg/kv/kvserver/asim/asim_test.go#L49) set to 10 and with --count = 100. TestAsimDeterministic (with --count = 5 due to test wait time): <img width="1053" alt="Screenshot 2023-07-27 at 8 09 50 AM" src="https://github.com/cockroachdb/cockroach/assets/56973754/24870322-0419-4646-9ca5-edf2dd76dd47"> TestAsimDeterministicZoneConf: <img width="1056" alt="Screenshot 2023-07-27 at 8 08 48 AM" src="https://github.com/cockroachdb/cockroach/assets/56973754/4d4b7918-3318-4535-a82e-2e98f172040c"> TestAsimDeterministicDiskFull: <img width="1006" alt="Screenshot 2023-07-27 at 8 08 19 AM" src="https://github.com/cockroachdb/cockroach/assets/56973754/fdbc7d77-2b40-432f-a599-cfc1eb3e1575"> example_zone_config: <img width="992" alt="Screenshot 2023-07-27 at 8 13 19 AM" src="https://github.com/cockroachdb/cockroach/assets/56973754/8b0f018f-9892-4268-a016-634c4ff8f26c"> example_fulldisk: <img width="1042" alt="Screenshot 2023-07-27 at 8 18 36 AM" src="https://github.com/cockroachdb/cockroach/assets/56973754/fe0fd3cc-1505-4510-8592-3c9b3ea759dc"> 107840: ci,ui: always install dependencies with pnpm r=nathanstilwell a=sjbarag Previously, the 'cluster-ui-release' and 'cluster-ui-release-next' GitHub Actions workflows would fail when no new version needed to be published. When setup-node performs its post-run actions, it attempts to cache the global pnpm store. Unfortunately, that caching step fails when no global pnpm store exists, like during a cache-miss when `pnpm install` never ran. This caused the entire workflow to be marked as failed, adding needless noise to anyone who changed a file in `pkg/ui/workspaces/cluster-ui` without bumping the package version. Always run `pnpm install`, so that the global pnpm store is always populated and cacheable. Release note: None Epic: none 107870: sql: fix TestSQLStatsCompactor flaky test r=koorosh a=koorosh This patch disables auto split and merge of ranges for the test because it causes extra wide scan to be counted by `kvInterceptor`. Also, it is defined to run test on system tenant only to have an access to `kv` specific cluster setting and because it should have an access to store level. Release note: None Resolves: cockroachdb#107067 107964: go.mod: bump datadriven to e384cf455877 r=kvoli,irfansharif a=wenyihu6 ``` e384cf4 Merge pull request cockroachdb#45 from wenyihu6/add-float64arr f4b8e52 add scan implementation for float64, time.Duration, and []float64 ``` Release Note: None Epic: None 107979: roachtest: improve flake detection for typeorm test r=rafiss a=rafiss fixes cockroachdb#100705 fixes cockroachdb#103182 fixes cockroachdb#103175 fixes cockroachdb#103172 Release note: None 107981: changefeedccl: remove two usages of disableDeclarativeSchemaChangesForTest r=miretskiy a=jayshrivastava This change removes two usages of `disableDeclarativeSchemaChangesForTest` from tests and replaces them with `maybeDisableDeclarativeSchemaChangesForTest` which will use the legacy schema changer 10% of the time. Release note: None Informs: cockroachdb#106906 Epic: None Co-authored-by: wenyihu6 <wenyi.hu@cockroachlabs.com> Co-authored-by: Sean Barag <barag@cockroachlabs.com> Co-authored-by: Andrii Vorobiov <and.vorobiov@gmail.com> Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com> Co-authored-by: Jayant Shrivastava <jayants@cockroachlabs.com>
diff --git a/.github/workflows/cluster-ui-release-next.yml b/.github/workflows/cluster-ui-release-next.yml
@@ -42,6 +42,16 @@ jobs:
       env:
         NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
 
+      # Always install node dependencies. It seems silly to do if we're not
+      # going to actually use them, but setup-node's post-run action attempts
+      # to save dependencies to a cache shared between GitHub actions. If the
+      # pnpm store directory doesn't exist (e.g. during a cache miss), that
+      # cache-saving step will fail and the entire job will be marked "failed"
+      # as a result. Installing dependencies is the canonical way to seed the
+      # pnpm store from-scratch.
+    - name: Install dependencies
+      run: pnpm install --frozen-lockfile
+
     - name: Check if version is published
       id: version-check
       shell: bash
@@ -62,7 +72,6 @@ jobs:
     - name: Build Cluster UI
       if: steps.version-check.outputs.published == 'no'
       run: |
-        pnpm install --frozen-lockfile
         bazel build //pkg/ui/workspaces/db-console/src/js:crdb-protobuf-client
         cp ../../../../_bazel/bin/pkg/ui/workspaces/db-console/src/js/protos.* ../db-console/src/js/
         pnpm build
diff --git a/.github/workflows/cluster-ui-release.yml b/.github/workflows/cluster-ui-release.yml
@@ -42,6 +42,16 @@ jobs:
       env:
         NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
 
+      # Always install node dependencies. It seems silly to do if we're not
+      # going to actually use them, but setup-node's post-run action attempts
+      # to save dependencies to a cache shared between GitHub actions. If the
+      # pnpm store directory doesn't exist (e.g. during a cache miss), that
+      # cache-saving step will fail and the entire job will be marked "failed"
+      # as a result. Installing dependencies is the canonical way to seed the
+      # pnpm store from-scratch.
+    - name: Install dependencies
+      run: pnpm install --frozen-lockfile
+
     - name: Check if version is published
       id: version-check
       shell: bash
@@ -67,7 +77,6 @@ jobs:
     - name: Build Cluster UI
       if: steps.version-check.outputs.published == 'no'
       run: |
-        pnpm install --frozen-lockfile
         bazel build //pkg/ui/workspaces/db-console/src/js:crdb-protobuf-client
         cp ../../../../_bazel/bin/pkg/ui/workspaces/db-console/src/js/protos.* ../db-console/src/js/
         pnpm build
diff --git a/DEPS.bzl b/DEPS.bzl
@@ -1532,10 +1532,10 @@ def go_deps():
         name = "com_github_cockroachdb_datadriven",
         build_file_proto_mode = "disable_global",
         importpath = "github.com/cockroachdb/datadriven",
-        sha256 = "f6de9c180e1ea80c602d98247b8e8fe89f491648ab1425417b9aca082697cbc0",
-        strip_prefix = "github.com/cockroachdb/datadriven@v1.0.3-0.20230413201302-be42291fc80f",
+        sha256 = "3f1ac51e496ac26ae297c9846bb78eb416fd50b7566b892fb7087af9273766fe",
+        strip_prefix = "github.com/cockroachdb/datadriven@v1.0.3-0.20230801171734-e384cf455877",
         urls = [
-            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/datadriven/com_github_cockroachdb_datadriven-v1.0.3-0.20230413201302-be42291fc80f.zip",
+            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/datadriven/com_github_cockroachdb_datadriven-v1.0.3-0.20230801171734-e384cf455877.zip",
         ],
     )
     go_repository(
diff --git a/build/bazelutil/distdir_files.bzl b/build/bazelutil/distdir_files.bzl
@@ -315,7 +315,7 @@ DISTDIR_FILES = {
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/cmux/com_github_cockroachdb_cmux-v0.0.0-20170110192607-30d10be49292.zip": "88f6f9cf33eb535658540b46f6222f029398e590a3ff9cc873d7d561ac6debf0",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/cockroach-go/v2/com_github_cockroachdb_cockroach_go_v2-v2.3.5.zip": "70860a2615f3df73f7d00b4801b1fffe30a3306ae1cda1e9bf5245bb74e86d9a",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/crlfmt/com_github_cockroachdb_crlfmt-v0.0.0-20221214225007-b2fc5c302548.zip": "fedc01bdd6d964da0425d5eaac8efadc951e78e13f102292cc0774197f09ab63",
-    "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/datadriven/com_github_cockroachdb_datadriven-v1.0.3-0.20230413201302-be42291fc80f.zip": "f6de9c180e1ea80c602d98247b8e8fe89f491648ab1425417b9aca082697cbc0",
+    "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/datadriven/com_github_cockroachdb_datadriven-v1.0.3-0.20230801171734-e384cf455877.zip": "3f1ac51e496ac26ae297c9846bb78eb416fd50b7566b892fb7087af9273766fe",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/errors/com_github_cockroachdb_errors-v1.9.2-0.20230612103643-126c21918793.zip": "d0ec6421e334689c9d023b8c27940c7a72c3110d90b3aa82267a101aa9858a9a",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/go-test-teamcity/com_github_cockroachdb_go_test_teamcity-v0.0.0-20191211140407-cff980ad0a55.zip": "bac30148e525b79d004da84d16453ddd2d5cd20528e9187f1d7dac708335674b",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/gostdlib/com_github_cockroachdb_gostdlib-v1.19.0.zip": "c4d516bcfe8c07b6fc09b8a9a07a95065b36c2855627cb3514e40c98f872b69e",
diff --git a/go.mod b/go.mod
@@ -111,7 +111,7 @@ require (
 	github.com/cockroachdb/cmux v0.0.0-20170110192607-30d10be49292
 	github.com/cockroachdb/cockroach-go/v2 v2.3.5
 	github.com/cockroachdb/crlfmt v0.0.0-20221214225007-b2fc5c302548
-	github.com/cockroachdb/datadriven v1.0.3-0.20230413201302-be42291fc80f
+	github.com/cockroachdb/datadriven v1.0.3-0.20230801171734-e384cf455877
 	github.com/cockroachdb/errors v1.9.2-0.20230612103643-126c21918793
 	github.com/cockroachdb/go-test-teamcity v0.0.0-20191211140407-cff980ad0a55
 	github.com/cockroachdb/gostdlib v1.19.0
diff --git a/go.sum b/go.sum
@@ -481,8 +481,8 @@ github.com/cockroachdb/crlfmt v0.0.0-20221214225007-b2fc5c302548 h1:i0bnjanlWAvM
 github.com/cockroachdb/crlfmt v0.0.0-20221214225007-b2fc5c302548/go.mod h1:qtkxNlt5i3rrdirfJE/bQeW/IeLajKexErv7jEIV+Uc=
 github.com/cockroachdb/datadriven v0.0.0-20190809214429-80d97fb3cbaa/go.mod h1:zn76sxSg3SzpJ0PPJaLDCu+Bu0Lg3sKTORVIj19EIF8=
 github.com/cockroachdb/datadriven v1.0.2/go.mod h1:a9RdTaap04u637JoCzcUoIcDmvwSUtcUFtT/C3kJlTU=
-github.com/cockroachdb/datadriven v1.0.3-0.20230413201302-be42291fc80f h1:otljaYPt5hWxV3MUfO5dFPFiOXg9CyG5/kCfayTqsJ4=
-github.com/cockroachdb/datadriven v1.0.3-0.20230413201302-be42291fc80f/go.mod h1:a9RdTaap04u637JoCzcUoIcDmvwSUtcUFtT/C3kJlTU=
+github.com/cockroachdb/datadriven v1.0.3-0.20230801171734-e384cf455877 h1:1MLK4YpFtIEo3ZtMA5C795Wtv5VuUnrXX7mQG+aHg6o=
+github.com/cockroachdb/datadriven v1.0.3-0.20230801171734-e384cf455877/go.mod h1:a9RdTaap04u637JoCzcUoIcDmvwSUtcUFtT/C3kJlTU=
 github.com/cockroachdb/errors v1.9.1/go.mod h1:2sxOtL2WIc096WSZqZ5h8fa17rdDq9HZOZLBCor4mBk=
 github.com/cockroachdb/errors v1.9.2-0.20230612103643-126c21918793 h1:2w2ycVjd8yt4G1rbU0K1OFYz4F/6iY9289KbDwSnRb0=
 github.com/cockroachdb/errors v1.9.2-0.20230612103643-126c21918793/go.mod h1:Ga5uDQye4MhjOl+lyfGROE0uNdhcImhW+qYVMsysmJk=
diff --git a/pkg/ccl/changefeedccl/alter_changefeed_test.go b/pkg/ccl/changefeedccl/alter_changefeed_test.go
@@ -1102,11 +1102,19 @@ func TestAlterChangefeedAddTargetsDuringSchemaChangeError(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	rnd, _ := randutil.NewPseudoRand()
+	rnd, seed := randutil.NewPseudoRand()
+	t.Logf("random seed: %d", seed)
 
 	testFn := func(t *testing.T, s TestServerWithSystem, f cdctest.TestFeedFactory) {
 		sqlDB := sqlutils.MakeSQLRunner(s.DB)
-		disableDeclarativeSchemaChangesForTest(t, sqlDB)
+		usingLegacySchemaChanger := maybeDisableDeclarativeSchemaChangesForTest(t, sqlDB, rnd)
+		// NB: For the `ALTER TABLE foo ADD COLUMN ... DEFAULT` schema change,
+		// the expected boundary is different depending on if we are using the
+		// legacy schema changer or not.
+		expectedBoundaryType := jobspb.ResolvedSpan_RESTART
+		if usingLegacySchemaChanger {
+			expectedBoundaryType = jobspb.ResolvedSpan_BACKFILL
+		}
 
 		knobs := s.TestingKnobs.
 			DistSQL.(*execinfra.TestingKnobs).
@@ -1181,10 +1189,9 @@ func TestAlterChangefeedAddTargetsDuringSchemaChangeError(t *testing.T) {
 				return true, nil
 			}
 
-			// A backfill begins when the backfill resolved event arrives, which has a
-			// timestamp such that all backfill spans have a timestamp of
-			// timestamp.Next()
-			if r.BoundaryType == jobspb.ResolvedSpan_BACKFILL {
+			// A backfill begins when the associated resolved event arrives, which has a
+			// timestamp such that all backfill spans have a timestamp of timestamp.Next().
+			if r.BoundaryType == expectedBoundaryType {
 				backfillTimestamp = r.Timestamp
 				return false, nil
 			}
diff --git a/pkg/ccl/changefeedccl/changefeed_test.go b/pkg/ccl/changefeedccl/changefeed_test.go
@@ -1866,7 +1866,8 @@ func TestChangefeedSchemaChangeBackfillCheckpoint(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	rnd, _ := randutil.NewPseudoRand()
+	rnd, seed := randutil.NewPseudoRand()
+	t.Logf("random seed: %d", seed)
 
 	// This test asserts that a second checkpoint made after resumption does its
 	// best to not lose information from the first checkpoint, therefore the
@@ -1876,7 +1877,14 @@ func TestChangefeedSchemaChangeBackfillCheckpoint(t *testing.T) {
 
 	testFn := func(t *testing.T, s TestServerWithSystem, f cdctest.TestFeedFactory) {
 		sqlDB := sqlutils.MakeSQLRunner(s.DB)
-		disableDeclarativeSchemaChangesForTest(t, sqlDB)
+		usingLegacySchemaChanger := maybeDisableDeclarativeSchemaChangesForTest(t, sqlDB, rnd)
+		// NB: For the `ALTER TABLE foo ADD COLUMN ... DEFAULT` schema change,
+		// the expected boundary is different depending on if we are using the
+		// legacy schema changer or not.
+		expectedBoundaryType := jobspb.ResolvedSpan_RESTART
+		if usingLegacySchemaChanger {
+			expectedBoundaryType = jobspb.ResolvedSpan_BACKFILL
+		}
 
 		knobs := s.TestingKnobs.
 			DistSQL.(*execinfra.TestingKnobs).
@@ -1957,10 +1965,10 @@ func TestChangefeedSchemaChangeBackfillCheckpoint(t *testing.T) {
 				return true, nil
 			}
 
-			// A backfill begins when the backfill resolved event arrives, which has a
+			// A backfill begins when the associated resolved event arrives, which has a
 			// timestamp such that all backfill spans have a timestamp of
-			// timestamp.Next()
-			if r.BoundaryType == jobspb.ResolvedSpan_BACKFILL {
+			// timestamp.Next().
+			if r.BoundaryType == expectedBoundaryType {
 				backfillTimestamp = r.Timestamp
 				return false, nil
 			}
diff --git a/pkg/ccl/changefeedccl/helpers_test.go b/pkg/ccl/changefeedccl/helpers_test.go
@@ -58,6 +58,21 @@ import (
 
 var testSinkFlushFrequency = 100 * time.Millisecond
 
+// disableDeclarativeSchemaChangesForTest will disable the declarative schema
+// changer with a probability of 10% using the provided SQL DB connection. This
+// returns true if the declarative schema changer is disabled.
+func maybeDisableDeclarativeSchemaChangesForTest(
+	t testing.TB, sqlDB *sqlutils.SQLRunner, rnd *rand.Rand,
+) bool {
+	disable := rnd.Float32() < 0.1
+	if disable {
+		t.Log("using legacy schema changer")
+		sqlDB.Exec(t, "SET use_declarative_schema_changer='off'")
+		sqlDB.Exec(t, "SET CLUSTER SETTING  sql.defaults.use_declarative_schema_changer='off'")
+	}
+	return disable
+}
+
 // disableDeclarativeSchemaChangesForTest tests that are disabled due to differences
 // in changefeed behaviour and are tracked by issue #80545.
 func disableDeclarativeSchemaChangesForTest(t testing.TB, sqlDB *sqlutils.SQLRunner) {
diff --git a/pkg/cmd/roachtest/tests/knex.go b/pkg/cmd/roachtest/tests/knex.go
@@ -22,7 +22,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-const supportedKnexTag = "2.0.0"
+const supportedKnexTag = "2.5.1"
 
 // This test runs one of knex's test suite against a single cockroach
 // node.
diff --git a/pkg/cmd/roachtest/tests/typeorm.go b/pkg/cmd/roachtest/tests/typeorm.go
@@ -14,6 +14,7 @@ import (
 	"context"
 	"fmt"
 	"regexp"
+	"strconv"
 	"strings"
 
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster"
@@ -166,12 +167,27 @@ func registerTypeORM(r registry.Registry) {
 		rawResults := result.Stdout + result.Stderr
 		t.L().Printf("Test Results: %s", rawResults)
 		if err != nil {
+			// We don't have a good way of parsing test results from javascript, so we
+			// use substring matching and regexp instead of using a blocklist like
+			// we use for other ORM tests.
+			numFailingRegex := regexp.MustCompile(`(\d+) failing`)
+			matches := numFailingRegex.FindStringSubmatch(rawResults)
+			numFailing, convErr := strconv.Atoi(matches[1])
+			if convErr != nil {
+				t.Fatal(convErr)
+			}
+
+			// One test is known to flake during setup.
+			if strings.Contains(rawResults, `"before each" hook for "should select specific columns":`) {
+				numFailing -= 1
+			}
+
+			// Tests are allowed to flake due to transaction retry errors.
 			txnRetryErrCount := strings.Count(rawResults, "restart transaction")
-			if strings.Contains(rawResults, "1 failing") && txnRetryErrCount == 1 {
-				err = nil
-			} else if strings.Contains(rawResults, "2 failing") && txnRetryErrCount == 2 {
+			if numFailing == txnRetryErrCount {
 				err = nil
 			}
+
 			if err != nil {
 				t.Fatal(err)
 			}
diff --git a/pkg/kv/kvserver/asim/asim_test.go b/pkg/kv/kvserver/asim/asim_test.go
@@ -39,17 +39,18 @@ func TestRunAllocatorSimulator(t *testing.T) {
 	sim.RunSim(ctx)
 }
 
-func TestAllocatorSimulatorDeterministic(t *testing.T) {
-	skip.WithIssue(t, 105904, "asim is non-deterministic")
+func TestAsimDeterministic(t *testing.T) {
+	skip.UnderRace(t, 105904, "asim is non-deterministic under race")
 	settings := config.DefaultSimulationSettings()
 
 	runs := 3
 	duration := 15 * time.Minute
 	settings.TickInterval = 2 * time.Second
 
-	stores := 7
+	stores := 21
 	replsPerRange := 3
-	replicasPerStore := 100
+	replicasPerStore := 600
+
 	// NB: We want 100 replicas per store, so the number of ranges required
 	// will be 1/3 of the total replicas.
 	ranges := (replicasPerStore * stores) / replsPerRange
diff --git a/pkg/kv/kvserver/asim/event/delayed_event.go b/pkg/kv/kvserver/asim/event/delayed_event.go
@@ -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)
 }
 
diff --git a/pkg/kv/kvserver/asim/gossip/exchange.go b/pkg/kv/kvserver/asim/gossip/exchange.go
@@ -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++ {
diff --git a/pkg/kv/kvserver/asim/state/change.go b/pkg/kv/kvserver/asim/state/change.go
@@ -477,25 +477,25 @@ func (rc *replicaChanger) Push(tick time.Time, change Change) (time.Time, bool)
 // Tick updates state changer to apply any changes that have occurred
 // between the last tick and this one.
 func (rc *replicaChanger) Tick(tick time.Time, state State) {
-	changeList := make(map[int]*pendingChange)
+	var changeList []*pendingChange
 
 	// NB: Add the smallest unit of time, in order to find all items in
 	// [smallest, tick].
 	pivot := &pendingChange{completeAt: tick.Add(time.Nanosecond)}
 	rc.completeAt.AscendLessThan(pivot, func(i btree.Item) bool {
 		nextChange, _ := i.(*pendingChange)
-		changeList[nextChange.ticket] = nextChange
+		changeList = append(changeList, nextChange)
 		return true
 	})
 
-	for ticket, nextChange := range changeList {
+	for _, nextChange := range changeList {
 		change := rc.pendingTickets[nextChange.ticket]
 		change.Apply(state)
 
 		// Cleanup the pending trackers for this ticket. This allows another
 		// change to be pushed for Range().
 		rc.completeAt.Delete(nextChange)
-		delete(rc.pendingTickets, ticket)
+		delete(rc.pendingTickets, nextChange.ticket)
 		delete(rc.pendingRange, change.Range())
 	}
 }
diff --git a/pkg/kv/kvserver/asim/state/impl.go b/pkg/kv/kvserver/asim/state/impl.go
@@ -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() })
@@ -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)
@@ -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
diff --git a/pkg/kv/kvserver/asim/state/new_state.go b/pkg/kv/kvserver/asim/state/new_state.go
@@ -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] }
diff --git a/pkg/kv/kvserver/asim/state/state.go b/pkg/kv/kvserver/asim/state/state.go
@@ -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.
diff --git a/pkg/kv/kvserver/asim/tests/testdata/example_zone_config b/pkg/kv/kvserver/asim/tests/testdata/example_zone_config
diff --git a/pkg/sql/sqlstats/persistedsqlstats/compaction_test.go b/pkg/sql/sqlstats/persistedsqlstats/compaction_test.go

Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@ import (`
`22`	`22`	`"github.com/stretchr/testify/require"`
`23`	`23`	`)`
`24`	`24`
`25`		`-const supportedKnexTag = "2.0.0"`
	`25`	`+const supportedKnexTag = "2.5.1"`
`26`	`26`
`27`	`27`	`// This test runs one of knex's test suite against a single cockroach`
`28`	`28`	`// node.`