Merge cockroachdb#108131

108131: persistedsqlstats: fix flakey TestPersistedSQLStatsRead r=j82w a=j82w

1. Test always failed on secondary tenant, because it was always connecting to system tenant.
2. Test was not properly validating in memory vs system table. The `IterateStatementStats` iterates both in memory and stats from the system table. New functions are added to verify it's actually in memory vs system table. The original check was added to verify the stats `IterateStatementStats` logic.

Fixes: cockroachdb#107804

Release note: None

Co-authored-by: j82w <[email protected]>

Author: craig[bot]

pkg/sql/sqlstats/persistedsqlstats/flush_test.go

@@ -38,24 +38,24 @@ import (
 
 type testCase struct {
 	query       string
-	fingerprint string
+	stmtNoConst string
 	count       int64
 }
 
 var testQueries = []testCase{
 	{
 		query:       "SELECT 1",
-		fingerprint: "SELECT _",
+		stmtNoConst: "SELECT _",
 		count:       3,
 	},
 	{
 		query:       "SELECT 1, 2, 3",
-		fingerprint: "SELECT _, _, _",
+		stmtNoConst: "SELECT _, _, _",
 		count:       10,
 	},
 	{
 		query:       "SELECT 1, 1 WHERE 1 < 10",
-		fingerprint: "SELECT _, _ WHERE _ < _",
+		stmtNoConst: "SELECT _, _ WHERE _ < _",
 		count:       7,
 	},
 }
@@ -120,8 +120,8 @@ func TestSQLStatsFlush(t *testing.T) {
 		// For each test case, we verify that it's being properly inserted exactly
 		// once and it is exactly executed tc.count number of times.
 		for _, tc := range testQueries {
-			verifyNumOfInsertedEntries(t, secondSQLConn, tc.fingerprint, firstServer.SQLInstanceID(), 1 /* expectedStmtEntryCnt */, 1 /* expectedTxnEntryCtn */)
-			verifyInsertedFingerprintExecCount(t, secondSQLConn, tc.fingerprint, fakeTime.getAggTimeTs(), firstServer.SQLInstanceID(), tc.count)
+			verifyNumOfInsertedEntries(t, secondSQLConn, tc.stmtNoConst, firstServer.SQLInstanceID(), 1 /* expectedStmtEntryCnt */, 1 /* expectedTxnEntryCtn */)
+			verifyInsertedFingerprintExecCount(t, secondSQLConn, tc.stmtNoConst, fakeTime.getAggTimeTs(), firstServer.SQLInstanceID(), tc.count)
 		}
 	}
 
@@ -145,10 +145,10 @@ func TestSQLStatsFlush(t *testing.T) {
 		verifyInMemoryStatsEmpty(t, testQueries, secondServerSQLStats)
 
 		for _, tc := range testQueries {
-			verifyNumOfInsertedEntries(t, secondSQLConn, tc.fingerprint, firstServer.SQLInstanceID(), 1 /* expectedStmtEntryCnt */, 1 /* expectedTxnEntryCtn */)
+			verifyNumOfInsertedEntries(t, secondSQLConn, tc.stmtNoConst, firstServer.SQLInstanceID(), 1 /* expectedStmtEntryCnt */, 1 /* expectedTxnEntryCtn */)
 			// The execution count is doubled here because we execute all of the
 			// statements here in the same aggregation interval.
-			verifyInsertedFingerprintExecCount(t, secondSQLConn, tc.fingerprint, fakeTime.getAggTimeTs(), firstServer.SQLInstanceID(), tc.count+tc.count-1 /* expectedCount */)
+			verifyInsertedFingerprintExecCount(t, secondSQLConn, tc.stmtNoConst, fakeTime.getAggTimeTs(), firstServer.SQLInstanceID(), tc.count+tc.count-1 /* expectedCount */)
 		}
 	}
 
@@ -172,8 +172,8 @@ func TestSQLStatsFlush(t *testing.T) {
 
 		for _, tc := range testQueries {
 			// We expect exactly 2 entries since we are in a different aggregation window.
-			verifyNumOfInsertedEntries(t, secondSQLConn, tc.fingerprint, firstServer.SQLInstanceID(), 2 /* expectedStmtEntryCnt */, 2 /* expectedTxnEntryCtn */)
-			verifyInsertedFingerprintExecCount(t, secondSQLConn, tc.fingerprint, fakeTime.getAggTimeTs(), firstServer.SQLInstanceID(), tc.count)
+			verifyNumOfInsertedEntries(t, secondSQLConn, tc.stmtNoConst, firstServer.SQLInstanceID(), 2 /* expectedStmtEntryCnt */, 2 /* expectedTxnEntryCtn */)
+			verifyInsertedFingerprintExecCount(t, secondSQLConn, tc.stmtNoConst, fakeTime.getAggTimeTs(), firstServer.SQLInstanceID(), tc.count)
 		}
 	}
 
@@ -196,10 +196,10 @@ func TestSQLStatsFlush(t *testing.T) {
 		// Ensure that we encode the correct node_id for the new entry and did not
 		// accidentally tamper the entries written by another server.
 		for _, tc := range testQueries {
-			verifyNumOfInsertedEntries(t, firstSQLConn, tc.fingerprint, secondServer.SQLInstanceID(), 1 /* expectedStmtEntryCnt */, 1 /* expectedTxnEntryCtn */)
-			verifyInsertedFingerprintExecCount(t, firstSQLConn, tc.fingerprint, fakeTime.getAggTimeTs(), secondServer.SQLInstanceID(), tc.count)
-			verifyNumOfInsertedEntries(t, secondSQLConn, tc.fingerprint, firstServer.SQLInstanceID(), 2 /* expectedStmtEntryCnt */, 2 /* expectedTxnEntryCtn */)
-			verifyInsertedFingerprintExecCount(t, secondSQLConn, tc.fingerprint, fakeTime.getAggTimeTs(), firstServer.SQLInstanceID(), tc.count)
+			verifyNumOfInsertedEntries(t, firstSQLConn, tc.stmtNoConst, secondServer.SQLInstanceID(), 1 /* expectedStmtEntryCnt */, 1 /* expectedTxnEntryCtn */)
+			verifyInsertedFingerprintExecCount(t, firstSQLConn, tc.stmtNoConst, fakeTime.getAggTimeTs(), secondServer.SQLInstanceID(), tc.count)
+			verifyNumOfInsertedEntries(t, secondSQLConn, tc.stmtNoConst, firstServer.SQLInstanceID(), 2 /* expectedStmtEntryCnt */, 2 /* expectedTxnEntryCtn */)
+			verifyInsertedFingerprintExecCount(t, secondSQLConn, tc.stmtNoConst, fakeTime.getAggTimeTs(), firstServer.SQLInstanceID(), tc.count)
 		}
 	}
 }
@@ -800,8 +800,8 @@ func verifyInMemoryStatsCorrectness(
 ) {
 	for _, tc := range tcs {
 		err := statsProvider.SQLStats.IterateStatementStats(context.Background(), sqlstats.IteratorOptions{}, func(ctx context.Context, statistics *appstatspb.CollectedStatementStatistics) error {
-			if tc.fingerprint == statistics.Key.Query {
-				require.Equal(t, tc.count, statistics.Stats.Count, "fingerprint: %s", tc.fingerprint)
+			if tc.stmtNoConst == statistics.Key.Query {
+				require.Equal(t, tc.count, statistics.Stats.Count, "fingerprint: %s", tc.stmtNoConst)
 			}
 
 			// All the queries should be under 1 minute
@@ -827,8 +827,8 @@ func verifyInMemoryStatsEmpty(
 ) {
 	for _, tc := range tcs {
 		err := statsProvider.SQLStats.IterateStatementStats(context.Background(), sqlstats.IteratorOptions{}, func(ctx context.Context, statistics *appstatspb.CollectedStatementStatistics) error {
-			if tc.fingerprint == statistics.Key.Query {
-				require.Equal(t, 0 /* expected */, statistics.Stats.Count, "fingerprint: %s", tc.fingerprint)
+			if tc.stmtNoConst == statistics.Key.Query {
+				require.Equal(t, 0 /* expected */, statistics.Stats.Count, "fingerprint: %s", tc.stmtNoConst)
 			}
 			return nil
 		})