diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index 9c7d4578bf97..dc0856d78044 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -2124,6 +2124,7 @@ GO_TARGETS = [ "//pkg/sql/sqlstats/insights/integration:integration_test", "//pkg/sql/sqlstats/insights:insights", "//pkg/sql/sqlstats/insights:insights_test", + "//pkg/sql/sqlstats/persistedsqlstats/sqlstatstestutil:sqlstatstestutil", "//pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil:sqlstatsutil", "//pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil:sqlstatsutil_test", "//pkg/sql/sqlstats/persistedsqlstats:persistedsqlstats", diff --git a/pkg/server/application_api/BUILD.bazel b/pkg/server/application_api/BUILD.bazel index 4f3399b73b3a..3b3602ea007c 100644 --- a/pkg/server/application_api/BUILD.bazel +++ b/pkg/server/application_api/BUILD.bazel @@ -67,6 +67,7 @@ go_test( "//pkg/sql/sessiondata", "//pkg/sql/sqlstats", "//pkg/sql/sqlstats/persistedsqlstats", + "//pkg/sql/sqlstats/persistedsqlstats/sqlstatstestutil", "//pkg/testutils", "//pkg/testutils/diagutils", "//pkg/testutils/serverutils", diff --git a/pkg/server/application_api/sql_stats_test.go b/pkg/server/application_api/sql_stats_test.go index 011b4853436f..ed41d55a2e4d 100644 --- a/pkg/server/application_api/sql_stats_test.go +++ b/pkg/server/application_api/sql_stats_test.go @@ -22,15 +22,19 @@ import ( "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/security/username" + "github.com/cockroachdb/cockroach/pkg/server" "github.com/cockroachdb/cockroach/pkg/server/apiconstants" "github.com/cockroachdb/cockroach/pkg/server/serverpb" "github.com/cockroachdb/cockroach/pkg/server/srvtestutils" + "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/spanconfig" "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/cockroach/pkg/sql/appstatspb" "github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants" "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" "github.com/cockroachdb/cockroach/pkg/sql/sqlstats" + "github.com/cockroachdb/cockroach/pkg/sql/sqlstats/persistedsqlstats" + "github.com/cockroachdb/cockroach/pkg/sql/sqlstats/persistedsqlstats/sqlstatstestutil" "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" "github.com/cockroachdb/cockroach/pkg/testutils/skip" @@ -1129,3 +1133,273 @@ func TestUnprivilegedUserResetIndexUsageStats(t *testing.T) { require.Contains(t, err.Error(), "requires admin privilege") } + +// TestCombinedStatementUsesCorrectSourceTable tests that requests read from +// the expected crdb_internal table given the table states. We have a lot of +// different tables that requests could potentially read from (in-memory, cached, +// system tables etc.), so we should sanity check that we are using the expected ones. +// given some simple table states. +func TestCombinedStatementUsesCorrectSourceTable(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + + ctx := context.Background() + + // Disable flushing sql stats so we can manually set the table states + // without worrying about unexpected stats appearing. + settings := cluster.MakeTestingClusterSettings() + statsKnobs := sqlstats.CreateTestingKnobs() + defaultMockInsertedAggTs := timeutil.Unix(1696906800, 0) + statsKnobs.StubTimeNow = func() time.Time { return defaultMockInsertedAggTs } + persistedsqlstats.SQLStatsFlushEnabled.Override(ctx, &settings.SV, false) + srv := serverutils.StartServerOnly(t, base.TestServerArgs{ + Settings: settings, + Knobs: base.TestingKnobs{ + SQLStatsKnobs: statsKnobs, + }, + }) + defer srv.Stopper().Stop(ctx) + + conn := sqlutils.MakeSQLRunner(srv.ApplicationLayer().SQLConn(t)) + conn.Exec(t, "SET application_name = $1", server.CrdbInternalStmtStatsCombined) + conn.Exec(t, "SET CLUSTER SETTING sql.stats.activity.flush.enabled = 'f'") + // Clear the in-memory stats so we only have the above app name. + // Then populate it with 1 query. + conn.Exec(t, "SELECT crdb_internal.reset_sql_stats()") + conn.Exec(t, "SELECT 1") + + type testCase struct { + name string + tableSetupFn func() error + expectedStmtsTable string + expectedTxnsTable string + reqs []serverpb.CombinedStatementsStatsRequest + isEmpty bool + returnInternal bool + } + + ie := srv.InternalExecutor().(*sql.InternalExecutor) + + defaultMockOneEach := func() error { + startTs := defaultMockInsertedAggTs + stmt := sqlstatstestutil.GetRandomizedCollectedStatementStatisticsForTest(t) + stmt.ID = 1 + stmt.AggregatedTs = startTs + stmt.Key.App = server.CrdbInternalStmtStatsPersisted + stmt.Key.TransactionFingerprintID = 1 + require.NoError(t, sqlstatstestutil.InsertMockedIntoSystemStmtStats(ctx, ie, &stmt, 1 /* nodeId */, nil)) + + stmt.Key.App = server.CrdbInternalStmtStatsCached + require.NoError(t, sqlstatstestutil.InsertMockedIntoSystemStmtActivity(ctx, ie, &stmt, nil)) + + txn := sqlstatstestutil.GetRandomizedCollectedTransactionStatisticsForTest(t) + txn.StatementFingerprintIDs = []appstatspb.StmtFingerprintID{1} + txn.TransactionFingerprintID = 1 + txn.AggregatedTs = startTs + txn.App = server.CrdbInternalTxnStatsPersisted + require.NoError(t, sqlstatstestutil.InsertMockedIntoSystemTxnStats(ctx, ie, &txn, 1, nil)) + txn.App = server.CrdbInternalTxnStatsCached + require.NoError(t, sqlstatstestutil.InsertMockedIntoSystemTxnActivity(ctx, ie, &txn, nil)) + + return nil + } + testCases := []testCase{ + { + name: "activity and persisted tables empty", + tableSetupFn: func() error { return nil }, + // We should attempt to read from the in-memory tables, since + // they are the last resort. + expectedStmtsTable: server.CrdbInternalStmtStatsCombined, + expectedTxnsTable: server.CrdbInternalTxnStatsCombined, + reqs: []serverpb.CombinedStatementsStatsRequest{ + {FetchMode: createTxnFetchMode(0)}, + }, + returnInternal: false, + }, + { + name: "all tables have data in selected range", + tableSetupFn: defaultMockOneEach, + expectedStmtsTable: server.CrdbInternalStmtStatsCached, + expectedTxnsTable: server.CrdbInternalTxnStatsCached, + reqs: []serverpb.CombinedStatementsStatsRequest{ + {Start: defaultMockInsertedAggTs.Unix()}, + { + Start: defaultMockInsertedAggTs.Unix(), + End: defaultMockInsertedAggTs.Unix(), + }, + }, + returnInternal: false, + }, + { + name: "all tables have data but no start range is provided", + tableSetupFn: defaultMockOneEach, + // When no date range is provided, we should default to reading from + // persisted or in-memory (whichever has data first). In this case the + // persisted table has data. + expectedStmtsTable: server.CrdbInternalStmtStatsPersisted, + expectedTxnsTable: server.CrdbInternalTxnStatsPersisted, + reqs: []serverpb.CombinedStatementsStatsRequest{ + {}, + {End: defaultMockInsertedAggTs.Unix()}, + }, + returnInternal: false, + }, + { + name: "all tables have data but not in the selected range", + tableSetupFn: defaultMockOneEach, + expectedStmtsTable: server.CrdbInternalStmtStatsCombined, + expectedTxnsTable: server.CrdbInternalTxnStatsCombined, + reqs: []serverpb.CombinedStatementsStatsRequest{ + {Start: defaultMockInsertedAggTs.Add(time.Hour).Unix()}, + {End: defaultMockInsertedAggTs.Truncate(time.Hour * 2).Unix()}, + }, + isEmpty: true, + returnInternal: false, + }, + { + name: "activity table has data in range with specified sort, fetchmode=txns", + tableSetupFn: defaultMockOneEach, + // For txn mode, we should not use the activity table for stmts. + expectedStmtsTable: server.CrdbInternalStmtStatsPersisted, + expectedTxnsTable: server.CrdbInternalTxnStatsCached, + // These sort options do exist on the activity table. + reqs: []serverpb.CombinedStatementsStatsRequest{ + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createTxnFetchMode(0)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createTxnFetchMode(1)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createTxnFetchMode(2)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createTxnFetchMode(3)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createTxnFetchMode(4)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createTxnFetchMode(5)}, + }, + returnInternal: false, + }, + { + name: "activity table has data in range with specified sort, fetchmode=stmts", + tableSetupFn: defaultMockOneEach, + expectedStmtsTable: server.CrdbInternalStmtStatsCached, + expectedTxnsTable: "", + // These sort options do exist on the activity table. + reqs: []serverpb.CombinedStatementsStatsRequest{ + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(0)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(1)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(2)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(3)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(4)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(5)}, + }, + returnInternal: false, + }, + { + name: "activity table has data in range, but selected sort is not on it, fetchmode=txns", + tableSetupFn: defaultMockOneEach, + expectedStmtsTable: server.CrdbInternalStmtStatsPersisted, + expectedTxnsTable: server.CrdbInternalTxnStatsPersisted, + // These sort options do not exist on the activity table. + reqs: []serverpb.CombinedStatementsStatsRequest{ + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createTxnFetchMode(6)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createTxnFetchMode(7)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createTxnFetchMode(8)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createTxnFetchMode(9)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createTxnFetchMode(10)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createTxnFetchMode(11)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createTxnFetchMode(12)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createTxnFetchMode(13)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createTxnFetchMode(14)}, + }, + returnInternal: false, + }, + { + name: "activity table has data in range, but selected sort is not on it, fetchmode=stmts", + tableSetupFn: defaultMockOneEach, + expectedStmtsTable: server.CrdbInternalStmtStatsPersisted, + expectedTxnsTable: "", + // These sort options do not exist on the activity table. + reqs: []serverpb.CombinedStatementsStatsRequest{ + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(6)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(7)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(8)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(9)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(10)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(11)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(12)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(13)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(14)}, + }, + returnInternal: false, + }, + { + name: "activity table has data in range with specified sort, fetchmode=stmts", + tableSetupFn: defaultMockOneEach, + expectedStmtsTable: server.CrdbInternalStmtStatsPersisted, + expectedTxnsTable: "", + // These sort options do exist on the activity table. + reqs: []serverpb.CombinedStatementsStatsRequest{ + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(0)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(1)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(2)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(3)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(4)}, + {Start: defaultMockInsertedAggTs.Unix(), FetchMode: createStmtFetchMode(5)}, + }, + returnInternal: true, + }, + } + + client := srv.ApplicationLayer().GetStatusClient(t) + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + require.NoError(t, tc.tableSetupFn()) + + defer func() { + // Reset tables. + conn.Exec(t, "SELECT crdb_internal.reset_sql_stats()") + conn.Exec(t, "SELECT crdb_internal.reset_activity_tables()") + conn.Exec(t, "SELECT 1") + }() + + for _, r := range tc.reqs { + conn.Exec(t, fmt.Sprintf("SET CLUSTER SETTING sql.stats.response.show_internal.enabled=%v", tc.returnInternal)) + resp, err := client.CombinedStatementStats(ctx, &r) + require.NoError(t, err) + + require.Equal(t, tc.expectedStmtsTable, resp.StmtsSourceTable, "req: %v", r) + require.Equal(t, tc.expectedTxnsTable, resp.TxnsSourceTable, "req: %v", r) + + if tc.isEmpty { + continue + } + + require.NotZero(t, len(resp.Statements), "req: %v", r) + // Verify we used the correct queries to return data. + require.Equal(t, tc.expectedStmtsTable, resp.Statements[0].Key.KeyData.App, "req: %v", r) + if tc.expectedTxnsTable == server.CrdbInternalTxnStatsCombined { + // For the combined query, we're using in-mem data and we set the + // app name there to the in-memory stmts table. + require.Equal(t, server.CrdbInternalStmtStatsCombined, resp.Transactions[0].StatsData.App, "req: %v", r) + } else if tc.expectedTxnsTable != "" { + require.NotZero(t, len(resp.Transactions)) + require.Equal(t, tc.expectedTxnsTable, resp.Transactions[0].StatsData.App, "req: %v", r) + } + } + + }) + } +} + +func createStmtFetchMode( + sort serverpb.StatsSortOptions, +) *serverpb.CombinedStatementsStatsRequest_FetchMode { + return &serverpb.CombinedStatementsStatsRequest_FetchMode{ + StatsType: serverpb.CombinedStatementsStatsRequest_StmtStatsOnly, + Sort: sort, + } +} +func createTxnFetchMode( + sort serverpb.StatsSortOptions, +) *serverpb.CombinedStatementsStatsRequest_FetchMode { + return &serverpb.CombinedStatementsStatsRequest_FetchMode{ + StatsType: serverpb.CombinedStatementsStatsRequest_TxnStatsOnly, + Sort: sort, + } +} diff --git a/pkg/server/combined_statement_stats.go b/pkg/server/combined_statement_stats.go index 8856bd84fa98..3474273a4b54 100644 --- a/pkg/server/combined_statement_stats.go +++ b/pkg/server/combined_statement_stats.go @@ -36,12 +36,12 @@ import ( const ( // Table sources. - crdbInternalStmtStatsCombined = "crdb_internal.statement_statistics" - crdbInternalStmtStatsPersisted = "crdb_internal.statement_statistics_persisted" - crdbInternalStmtStatsCached = "crdb_internal.statement_activity" - crdbInternalTxnStatsCombined = "crdb_internal.transaction_statistics" - crdbInternalTxnStatsPersisted = "crdb_internal.transaction_statistics_persisted" - crdbInternalTxnStatsCached = "crdb_internal.transaction_activity" + CrdbInternalStmtStatsCombined = "crdb_internal.statement_statistics" + CrdbInternalStmtStatsPersisted = "crdb_internal.statement_statistics_persisted" + CrdbInternalStmtStatsCached = "crdb_internal.statement_activity" + CrdbInternalTxnStatsCombined = "crdb_internal.transaction_statistics" + CrdbInternalTxnStatsPersisted = "crdb_internal.transaction_statistics_persisted" + CrdbInternalTxnStatsCached = "crdb_internal.transaction_activity" // Sorts sortSvcLatDesc = `(statistics -> 'statistics' -> 'svcLat' ->> 'mean')::FLOAT DESC` @@ -227,6 +227,11 @@ func activityTablesHaveFullData( return false, nil } + // Top Activity table doesn't store internal data. + if SQLStatsShowInternal.Get(&settings.SV) { + return false, nil + } + if (limit > 0 && !isLimitOnActivityTable(limit)) || !isSortOptionOnActivityTable(order) { return false, nil } @@ -423,8 +428,8 @@ FROM %s %s`, table, whereClause) // We return statement info for both req modes (statements only and transactions only), // since statements are also returned for transactions only mode. stmtsRuntime = 0 - if activityTableHasAllData { - stmtSourceTable = crdbInternalStmtStatsCached + if activityTableHasAllData && (req.FetchMode == nil || req.FetchMode.StatsType == serverpb.CombinedStatementsStatsRequest_StmtStatsOnly) { + stmtSourceTable = CrdbInternalStmtStatsCached stmtsRuntime, err = getRuntime(stmtSourceTable, createActivityTableQuery) if err != nil { return 0, 0, nil, stmtSourceTable, "", err @@ -436,7 +441,7 @@ FROM %s %s`, table, whereClause) } // If there are no results from the activity table, retrieve the data from the persisted table. if stmtsRuntime == 0 { - stmtSourceTable = crdbInternalStmtStatsPersisted + tableSuffix + stmtSourceTable = CrdbInternalStmtStatsPersisted + tableSuffix stmtsRuntime, err = getRuntime(stmtSourceTable, createStatsTableQuery) if err != nil { return 0, 0, nil, stmtSourceTable, "", err @@ -449,7 +454,7 @@ FROM %s %s`, table, whereClause) // If there are no results from the persisted table, retrieve the data from the combined view // with data in-memory. if stmtsRuntime == 0 { - stmtSourceTable = crdbInternalStmtStatsCombined + stmtSourceTable = CrdbInternalStmtStatsCombined stmtsRuntime, err = getRuntime(stmtSourceTable, createStatsTableQuery) if err != nil { return 0, 0, nil, stmtSourceTable, "", err @@ -463,7 +468,7 @@ FROM %s %s`, table, whereClause) txnsRuntime = 0 if req.FetchMode == nil || req.FetchMode.StatsType != serverpb.CombinedStatementsStatsRequest_StmtStatsOnly { if activityTableHasAllData { - txnSourceTable = crdbInternalTxnStatsCached + txnSourceTable = CrdbInternalTxnStatsCached txnsRuntime, err = getRuntime(txnSourceTable, createActivityTableQuery) if err != nil { return 0, 0, nil, stmtSourceTable, txnSourceTable, err @@ -471,7 +476,7 @@ FROM %s %s`, table, whereClause) } // If there are no results from the activity table, retrieve the data from the persisted table. if txnsRuntime == 0 { - txnSourceTable = crdbInternalTxnStatsPersisted + tableSuffix + txnSourceTable = CrdbInternalTxnStatsPersisted + tableSuffix txnsRuntime, err = getRuntime(txnSourceTable, createStatsTableQuery) if err != nil { return 0, 0, nil, stmtSourceTable, txnSourceTable, err @@ -480,7 +485,7 @@ FROM %s %s`, table, whereClause) // If there are no results from the persisted table, retrieve the data from the combined view // with data in-memory. if txnsRuntime == 0 { - txnSourceTable = crdbInternalTxnStatsCombined + txnSourceTable = CrdbInternalTxnStatsCombined txnsRuntime, err = getRuntime(txnSourceTable, createStatsTableQuery) if err != nil { return 0, 0, nil, stmtSourceTable, txnSourceTable, err @@ -730,7 +735,7 @@ FROM (SELECT fingerprint_id, fingerprint_id, app_name) %s %s`, - crdbInternalStmtStatsCached, + CrdbInternalStmtStatsCached, "combined-stmts-activity-by-interval", whereClause, args, @@ -750,7 +755,7 @@ FROM (SELECT fingerprint_id, ctx, ie, queryFormat, - crdbInternalStmtStatsPersisted+tableSuffix, + CrdbInternalStmtStatsPersisted+tableSuffix, "combined-stmts-persisted-by-interval", whereClause, args, @@ -769,7 +774,7 @@ FROM (SELECT fingerprint_id, ctx, ie, queryFormat, - crdbInternalStmtStatsCombined, + CrdbInternalStmtStatsCombined, "combined-stmts-with-memory-by-interval", whereClause, args, @@ -906,7 +911,7 @@ FROM (SELECT app_name, ctx, ie, queryFormat, - crdbInternalTxnStatsCached, + CrdbInternalTxnStatsCached, "combined-txns-activity-by-interval", whereClause, args, @@ -930,7 +935,7 @@ FROM (SELECT app_name, ctx, ie, queryFormat, - crdbInternalTxnStatsPersisted+tableSuffix, + CrdbInternalTxnStatsPersisted+tableSuffix, "combined-txns-persisted-by-interval", whereClause, args, @@ -949,7 +954,7 @@ FROM (SELECT app_name, ctx, ie, queryFormat, - crdbInternalTxnStatsCombined, + CrdbInternalTxnStatsCombined, "combined-txns-with-memory-by-interval", whereClause, args, @@ -1044,7 +1049,7 @@ GROUP BY query := fmt.Sprintf( queryFormat, - crdbInternalStmtStatsPersisted+tableSuffix, + CrdbInternalStmtStatsPersisted+tableSuffix, whereClause) it, err = ie.QueryIteratorEx(ctx, "console-combined-stmts-persisted-for-txn", nil, sessiondata.NodeUserSessionDataOverride, query, args...) @@ -1060,7 +1065,7 @@ GROUP BY if err != nil { return nil, srverrors.ServerError(ctx, err) } - query = fmt.Sprintf(queryFormat, crdbInternalStmtStatsCombined, whereClause) + query = fmt.Sprintf(queryFormat, CrdbInternalStmtStatsCombined, whereClause) it, err = ie.QueryIteratorEx(ctx, "console-combined-stmts-with-memory-for-txn", nil, sessiondata.NodeUserSessionDataOverride, query, args...) diff --git a/pkg/server/statement_details.go b/pkg/server/statement_details.go index a7f9e155b246..0b46171dcb83 100644 --- a/pkg/server/statement_details.go +++ b/pkg/server/statement_details.go @@ -254,7 +254,7 @@ LIMIT 1`, whereClause), args...) sessiondata.NodeUserSessionDataOverride, fmt.Sprintf( queryFormat, - crdbInternalStmtStatsPersisted+tableSuffix, + CrdbInternalStmtStatsPersisted+tableSuffix, whereClause), args...) if err != nil { return statement, srverrors.ServerError(ctx, err) @@ -266,7 +266,7 @@ LIMIT 1`, whereClause), args...) if row.Len() == 0 { row, err = ie.QueryRowEx(ctx, "combined-stmts-details-total-with-memory", nil, sessiondata.NodeUserSessionDataOverride, - fmt.Sprintf(queryFormat, crdbInternalStmtStatsCombined, whereClause), args...) + fmt.Sprintf(queryFormat, CrdbInternalStmtStatsCombined, whereClause), args...) if err != nil { return statement, srverrors.ServerError(ctx, err) } @@ -368,7 +368,7 @@ LIMIT $%d`, whereClause, len(args)), } query = fmt.Sprintf( queryFormat, - crdbInternalStmtStatsPersisted+tableSuffix, + CrdbInternalStmtStatsPersisted+tableSuffix, whereClause, len(args)) @@ -384,7 +384,7 @@ LIMIT $%d`, whereClause, len(args)), // with data in-memory. if !it.HasResults() { err = closeIterator(it, err) - query = fmt.Sprintf(queryFormat, crdbInternalStmtStatsCombined, whereClause, len(args)) + query = fmt.Sprintf(queryFormat, CrdbInternalStmtStatsCombined, whereClause, len(args)) it, err = ie.QueryIteratorEx(ctx, "console-combined-stmts-details-by-aggregated-timestamp-with-memory", nil, sessiondata.NodeUserSessionDataOverride, query, args...) if err != nil { @@ -580,7 +580,7 @@ LIMIT $%d`, whereClause, len(args)), args...) // with data in-memory. if !it.HasResults() { err = closeIterator(it, err) - query = fmt.Sprintf(queryFormat, crdbInternalStmtStatsCombined, whereClause, len(args)) + query = fmt.Sprintf(queryFormat, CrdbInternalStmtStatsCombined, whereClause, len(args)) it, iterErr = ie.QueryIteratorEx(ctx, "console-combined-stmts-details-by-plan-hash-with-memory", nil, sessiondata.NodeUserSessionDataOverride, query, args...) if iterErr != nil { diff --git a/pkg/sql/sqlstats/persistedsqlstats/sqlstatstestutil/BUILD.bazel b/pkg/sql/sqlstats/persistedsqlstats/sqlstatstestutil/BUILD.bazel new file mode 100644 index 000000000000..ee756fa9d4e0 --- /dev/null +++ b/pkg/sql/sqlstats/persistedsqlstats/sqlstatstestutil/BUILD.bazel @@ -0,0 +1,16 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "sqlstatstestutil", + srcs = ["testutils.go"], + importpath = "github.com/cockroachdb/cockroach/pkg/sql/sqlstats/persistedsqlstats/sqlstatstestutil", + visibility = ["//visibility:public"], + deps = [ + "//pkg/base", + "//pkg/sql", + "//pkg/sql/appstatspb", + "//pkg/sql/sem/tree", + "//pkg/sql/sessiondata", + "//pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil", + ], +) diff --git a/pkg/sql/sqlstats/persistedsqlstats/sqlstatstestutil/testutils.go b/pkg/sql/sqlstats/persistedsqlstats/sqlstatstestutil/testutils.go new file mode 100644 index 000000000000..dd2b7775f447 --- /dev/null +++ b/pkg/sql/sqlstats/persistedsqlstats/sqlstatstestutil/testutils.go @@ -0,0 +1,272 @@ +// Copyright 2023 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package sqlstatstestutil + +import ( + "context" + "reflect" + "testing" + "time" + + "github.com/cockroachdb/cockroach/pkg/base" + "github.com/cockroachdb/cockroach/pkg/sql" + "github.com/cockroachdb/cockroach/pkg/sql/appstatspb" + "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" + "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" + "github.com/cockroachdb/cockroach/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil" +) + +// GetRandomizedCollectedStatementStatisticsForTest returns a +// appstatspb.CollectedStatementStatistics with its fields randomly filled. +func GetRandomizedCollectedStatementStatisticsForTest( + t *testing.T, +) (result appstatspb.CollectedStatementStatistics) { + data := sqlstatsutil.GenRandomData() + sqlstatsutil.FillObject(t, reflect.ValueOf(&result), &data) + + return result +} + +// GetRandomizedCollectedTransactionStatisticsForTest returns a +// appstatspb.CollectedTransactionStatistics with its fields randomly filled. +func GetRandomizedCollectedTransactionStatisticsForTest( + t *testing.T, +) (result appstatspb.CollectedTransactionStatistics) { + data := sqlstatsutil.GenRandomData() + sqlstatsutil.FillObject(t, reflect.ValueOf(&result), &data) + + return result +} + +func InsertMockedIntoSystemStmtStats( + ctx context.Context, + ie *sql.InternalExecutor, + stmtStats *appstatspb.CollectedStatementStatistics, + nodeID base.SQLInstanceID, + aggInterval *time.Duration, +) error { + if stmtStats == nil { + return nil + } + + aggIntervalVal := time.Hour + if aggInterval != nil { + aggIntervalVal = *aggInterval + } + + stmtFingerprint := sqlstatsutil.EncodeUint64ToBytes(uint64(stmtStats.ID)) + txnFingerprint := sqlstatsutil.EncodeUint64ToBytes(uint64(stmtStats.Key.TransactionFingerprintID)) + planHash := sqlstatsutil.EncodeUint64ToBytes(stmtStats.Key.PlanHash) + + metadataJSON, err := sqlstatsutil.BuildStmtMetadataJSON(stmtStats) + if err != nil { + return err + } + + statisticsJSON, err := sqlstatsutil.BuildStmtStatisticsJSON(&stmtStats.Stats) + if err != nil { + return err + } + statistics := tree.NewDJSON(statisticsJSON) + + plan := tree.NewDJSON(sqlstatsutil.ExplainTreePlanNodeToJSON(&stmtStats.Stats.SensitiveInfo.MostRecentPlanDescription)) + + metadata := tree.NewDJSON(metadataJSON) + + _, err = ie.ExecEx(ctx, "insert-mock-stmt-stats", nil, sessiondata.NodeUserSessionDataOverride, + `UPSERT INTO system.statement_statistics +VALUES ($1 ,$2, $3, $4, $5, $6, $7, $8, $9, $10)`, + stmtStats.AggregatedTs, // aggregated_ts + stmtFingerprint, // fingerprint_id + txnFingerprint, // transaction_fingerprint_id + planHash, // plan_hash + stmtStats.Key.App, // app_name + nodeID, // node_id + aggIntervalVal, // agg_interval + metadata, // metadata + statistics, // statistics + plan, // plan + ) + + return err +} + +func InsertMockedIntoSystemTxnStats( + ctx context.Context, + ie *sql.InternalExecutor, + stats *appstatspb.CollectedTransactionStatistics, + nodeID base.SQLInstanceID, + aggInterval *time.Duration, +) error { + if stats == nil { + return nil + } + + aggIntervalVal := time.Hour + if aggInterval != nil { + aggIntervalVal = *aggInterval + } + + txnFingerprint := sqlstatsutil.EncodeUint64ToBytes(uint64(stats.TransactionFingerprintID)) + + statisticsJSON, err := sqlstatsutil.BuildTxnStatisticsJSON(stats) + if err != nil { + return err + } + statistics := tree.NewDJSON(statisticsJSON) + + metadataJSON, err := sqlstatsutil.BuildTxnMetadataJSON(stats) + if err != nil { + return err + } + metadata := tree.NewDJSON(metadataJSON) + aggregatedTs := stats.AggregatedTs + + _, err = ie.ExecEx(ctx, "insert-mock-txn-stats", nil, sessiondata.NodeUserSessionDataOverride, + ` UPSERT INTO system.transaction_statistics +VALUES ($1 ,$2, $3, $4, $5, $6, $7)`, + aggregatedTs, // aggregated_ts + txnFingerprint, // fingerprint_id + stats.App, // app_name + nodeID, // node_id + aggIntervalVal, // agg_interval + metadata, // metadata + statistics, // statistics + ) + + return err +} + +func InsertMockedIntoSystemStmtActivity( + ctx context.Context, + ie *sql.InternalExecutor, + stmtStats *appstatspb.CollectedStatementStatistics, + aggInterval *time.Duration, +) error { + if stmtStats == nil { + return nil + } + + aggIntervalVal := time.Hour + if aggInterval != nil { + aggIntervalVal = *aggInterval + } + + stmtFingerprint := sqlstatsutil.EncodeUint64ToBytes(uint64(stmtStats.ID)) + txnFingerprint := sqlstatsutil.EncodeUint64ToBytes(uint64(stmtStats.Key.TransactionFingerprintID)) + planHash := sqlstatsutil.EncodeUint64ToBytes(stmtStats.Key.PlanHash) + + statisticsJSON, err := sqlstatsutil.BuildStmtStatisticsJSON(&stmtStats.Stats) + if err != nil { + return err + } + statistics := tree.NewDJSON(statisticsJSON) + + plan := tree.NewDJSON(sqlstatsutil.ExplainTreePlanNodeToJSON(&stmtStats.Stats.SensitiveInfo.MostRecentPlanDescription)) + + metadataJSON, err := sqlstatsutil.BuildStmtDetailsMetadataJSON( + &appstatspb.AggregatedStatementMetadata{ + Query: stmtStats.Key.Query, + FormattedQuery: "", + QuerySummary: "", + StmtType: "", + AppNames: []string{stmtStats.Key.App}, + Databases: []string{stmtStats.Key.Database}, + ImplicitTxn: false, + DistSQLCount: 0, + FailedCount: 0, + FullScanCount: 0, + VecCount: 0, + TotalCount: 0, + }) + if err != nil { + return err + } + metadata := tree.NewDJSON(metadataJSON) + + _, err = ie.ExecEx(ctx, "insert-mock-stmt-activity", nil, sessiondata.NodeUserSessionDataOverride, + `UPSERT INTO system.statement_activity +VALUES ($1 ,$2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17)`, + stmtStats.AggregatedTs, // aggregated_ts + stmtFingerprint, // fingerprint_id + txnFingerprint, // transaction_fingerprint_id + planHash, // plan_hash + stmtStats.Key.App, // app_name + aggIntervalVal, // agg_interval + metadata, // metadata + statistics, // statistics + plan, // plan + // TODO allow these values to be mocked. No need for them right now. + []string{}, // index_recommendations + 1, // execution_count + 1, // execution_total_seconds + 1, // execution_total_cluster_seconds + 1, // contention_time_avg_seconds + 1, // cpu_sql_avg_nanos + 1, // service_latency_avg_seconds + 1, // service_latency_p99_seconds + ) + + return err +} + +func InsertMockedIntoSystemTxnActivity( + ctx context.Context, + ie *sql.InternalExecutor, + stats *appstatspb.CollectedTransactionStatistics, + aggInterval *time.Duration, +) error { + if stats == nil { + return nil + } + + aggIntervalVal := time.Hour + if aggInterval != nil { + aggIntervalVal = *aggInterval + } + + txnFingerprint := sqlstatsutil.EncodeUint64ToBytes(uint64(stats.TransactionFingerprintID)) + + statisticsJSON, err := sqlstatsutil.BuildTxnStatisticsJSON(stats) + if err != nil { + return err + } + statistics := tree.NewDJSON(statisticsJSON) + + metadataJSON, err := sqlstatsutil.BuildTxnMetadataJSON(stats) + if err != nil { + return err + } + metadata := tree.NewDJSON(metadataJSON) + aggregatedTs := stats.AggregatedTs + + _, err = ie.ExecEx(ctx, "insert-mock-txn-activity", nil, sessiondata.NodeUserSessionDataOverride, + ` UPSERT INTO system.transaction_activity +VALUES ($1 ,$2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14)`, + aggregatedTs, // aggregated_ts + txnFingerprint, // fingerprint_id + stats.App, // app_name + aggIntervalVal, // agg_interval + metadata, // metadata + statistics, // statistics + // TODO (xinhaoz) allow mocking of these fields. Not necessary at the moment. + "", // query + 1, // execution_count + 1, // execution_total_seconds + 1, // execution_total_cluster_seconds + 1, // contention_time_avg_seconds + 1, // cpu_sql_avg_nanos + 1, // service_latency_avg_seconds + 1, // service_latency_p99_seconds + ) + + return err +} diff --git a/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_encoding_test.go b/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_encoding_test.go index ae3d8e49dcac..6265d4c0e4d9 100644 --- a/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_encoding_test.go +++ b/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_encoding_test.go @@ -36,7 +36,7 @@ func TestSQLStatsJsonEncoding(t *testing.T) { defer log.Scope(t).Close(t) t.Run("statement_statistics", func(t *testing.T) { - data := genRandomData() + data := GenRandomData() input := appstatspb.CollectedStatementStatistics{} expectedMetadataStrTemplate := ` @@ -200,7 +200,7 @@ func TestSQLStatsJsonEncoding(t *testing.T) { expectedMetadataStr := fillTemplate(t, expectedMetadataStrTemplate, data) expectedStatisticsStr := fillTemplate(t, expectedStatisticsStrTemplate, data) - fillObject(t, reflect.ValueOf(&input), &data) + FillObject(t, reflect.ValueOf(&input), &data) actualMetadataJSON, err := BuildStmtMetadataJSON(&input) require.NoError(t, err) @@ -228,7 +228,7 @@ func TestSQLStatsJsonEncoding(t *testing.T) { // new parameter, so this test is to confirm that all other parameters will be set and // the new one will be empty, without breaking the decoding process. t.Run("statement_statistics with new parameter", func(t *testing.T) { - data := genRandomData() + data := GenRandomData() expectedStatistics := appstatspb.CollectedStatementStatistics{} expectedMetadataStrTemplate := ` @@ -385,7 +385,7 @@ func TestSQLStatsJsonEncoding(t *testing.T) { fillTemplate(t, expectedMetadataStrTemplate, data) fillTemplate(t, expectedStatisticsStrTemplate, data) - fillObject(t, reflect.ValueOf(&expectedStatistics), &data) + FillObject(t, reflect.ValueOf(&expectedStatistics), &data) actualMetadataJSON, err := BuildStmtMetadataJSON(&expectedStatistics) require.NoError(t, err) @@ -412,7 +412,7 @@ func TestSQLStatsJsonEncoding(t *testing.T) { }) t.Run("transaction_statistics", func(t *testing.T) { - data := genRandomData() + data := GenRandomData() input := appstatspb.CollectedTransactionStatistics{ StatementFingerprintIDs: []appstatspb.StmtFingerprintID{ @@ -552,7 +552,7 @@ func TestSQLStatsJsonEncoding(t *testing.T) { } ` expectedStatisticsStr := fillTemplate(t, expectedStatisticsStrTemplate, data) - fillObject(t, reflect.ValueOf(&input), &data) + FillObject(t, reflect.ValueOf(&input), &data) actualMetadataJSON, err := BuildTxnMetadataJSON(&input) require.NoError(t, err) @@ -574,7 +574,7 @@ func TestSQLStatsJsonEncoding(t *testing.T) { }) t.Run("statement aggregated metadata", func(t *testing.T) { - data := genRandomData() + data := GenRandomData() input := appstatspb.AggregatedStatementMetadata{} @@ -596,7 +596,7 @@ func TestSQLStatsJsonEncoding(t *testing.T) { } ` expectedAggregatedMetadataStr := fillTemplate(t, expectedAggregatedMetadataStrTemplate, data) - fillObject(t, reflect.ValueOf(&input), &data) + FillObject(t, reflect.ValueOf(&input), &data) actualMetadataJSON, err := BuildStmtDetailsMetadataJSON(&input) require.NoError(t, err) diff --git a/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/testutils.go b/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/testutils.go index 574cc19159cb..2057e5132ece 100644 --- a/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/testutils.go +++ b/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/testutils.go @@ -20,22 +20,10 @@ import ( "text/template" "time" - "github.com/cockroachdb/cockroach/pkg/sql/appstatspb" "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/stretchr/testify/require" ) -// GetRandomizedCollectedStatementStatisticsForTest returns a -// appstatspb.CollectedStatementStatistics with its fields randomly filled. -func GetRandomizedCollectedStatementStatisticsForTest( - t *testing.T, -) (result appstatspb.CollectedStatementStatistics) { - data := genRandomData() - fillObject(t, reflect.ValueOf(&result), &data) - - return result -} - type randomData struct { Bool bool String string @@ -48,7 +36,7 @@ type randomData struct { var alphabet = []rune("abcdefghijklmkopqrstuvwxyz") -func genRandomData() randomData { +func GenRandomData() randomData { r := randomData{} r.Bool = rand.Float64() > 0.5 @@ -134,7 +122,7 @@ var fieldBlacklist = map[string]struct{}{ "AggregationInterval": {}, } -func fillObject(t *testing.T, val reflect.Value, data *randomData) { +func FillObject(t *testing.T, val reflect.Value, data *randomData) { // Do not set the fields that are not being encoded as json. if val.Kind() != reflect.Ptr { t.Fatal("not a pointer type") @@ -179,7 +167,7 @@ func fillObject(t *testing.T, val reflect.Value, data *randomData) { continue } - fillObject(t, fieldAddr, data) + FillObject(t, fieldAddr, data) } } default: diff --git a/pkg/sql/sqlstats/sslocal/BUILD.bazel b/pkg/sql/sqlstats/sslocal/BUILD.bazel index 0a9628c1dea3..bb5db6a70082 100644 --- a/pkg/sql/sqlstats/sslocal/BUILD.bazel +++ b/pkg/sql/sqlstats/sslocal/BUILD.bazel @@ -60,7 +60,7 @@ go_test( "//pkg/sql/sqlstats", "//pkg/sql/sqlstats/insights", "//pkg/sql/sqlstats/persistedsqlstats", - "//pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil", + "//pkg/sql/sqlstats/persistedsqlstats/sqlstatstestutil", "//pkg/testutils", "//pkg/testutils/serverutils", "//pkg/testutils/sqlutils", diff --git a/pkg/sql/sqlstats/sslocal/sql_stats_test.go b/pkg/sql/sqlstats/sslocal/sql_stats_test.go index 0acbb5e4275b..73150632a9ba 100644 --- a/pkg/sql/sqlstats/sslocal/sql_stats_test.go +++ b/pkg/sql/sqlstats/sslocal/sql_stats_test.go @@ -35,7 +35,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sqlstats" "github.com/cockroachdb/cockroach/pkg/sql/sqlstats/insights" "github.com/cockroachdb/cockroach/pkg/sql/sqlstats/persistedsqlstats" - "github.com/cockroachdb/cockroach/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil" + "github.com/cockroachdb/cockroach/pkg/sql/sqlstats/persistedsqlstats/sqlstatstestutil" "github.com/cockroachdb/cockroach/pkg/sql/sqlstats/sslocal" "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" @@ -62,7 +62,7 @@ func TestStmtStatsBulkIngestWithRandomMetadata(t *testing.T) { for i := 0; i < 50; i++ { var stats serverpb.StatementsResponse_CollectedStatementStatistics - randomData := sqlstatsutil.GetRandomizedCollectedStatementStatisticsForTest(t) + randomData := sqlstatstestutil.GetRandomizedCollectedStatementStatisticsForTest(t) stats.Key.KeyData = randomData.Key testData = append(testData, stats) } diff --git a/pkg/ui/workspaces/cluster-ui/src/sqlActivity/util.spec.tsx b/pkg/ui/workspaces/cluster-ui/src/sqlActivity/util.spec.tsx index bbc996096947..64f8af7f664a 100644 --- a/pkg/ui/workspaces/cluster-ui/src/sqlActivity/util.spec.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/sqlActivity/util.spec.tsx @@ -71,7 +71,7 @@ describe("filterStatementsData", () => { filterAndCheckStmts(stmtsRaw, {}, "giraffe", expectedIDs); }); - it("should show non-internal statements when no app filters are applied", () => { + it("should show internal statements when no app filters are applied", () => { const stmtsRaw = [ { id: 1, app: "hello" }, { id: 2, app: "$ internal hello" }, @@ -86,7 +86,7 @@ describe("filterStatementsData", () => { ); const filters: Filters = {}; - const expected = [1, 4, 5]; + const expected = [1, 2, 3, 4, 5]; filterAndCheckStmts(stmtsRaw, filters, null, expected); }); diff --git a/pkg/ui/workspaces/cluster-ui/src/sqlActivity/util.tsx b/pkg/ui/workspaces/cluster-ui/src/sqlActivity/util.tsx index dea09898f749..ab348eba8222 100644 --- a/pkg/ui/workspaces/cluster-ui/src/sqlActivity/util.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/sqlActivity/util.tsx @@ -97,7 +97,7 @@ export function filterStatementsData( INTERNAL_APP_NAME_PREFIX, ); return ( - (!appNames?.length && !isInternal) || + !appNames?.length || (includeInternalApps && isInternal) || appNames?.includes( statement.applicationName ? statement.applicationName : unset, diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.spec.ts b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.spec.ts index b7a415707c89..525a24e7831d 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.spec.ts +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.spec.ts @@ -44,7 +44,7 @@ describe("getStatementsByFingerprintId", () => { const txData = data.transactions as Transaction[]; describe("Filter transactions", () => { - it("show non internal if no filters applied", () => { + it("show internal if no filters applied", () => { const filter: Filters = { app: "", timeNumber: "0", @@ -61,7 +61,7 @@ describe("Filter transactions", () => { nodeRegions, false, ).transactions.length, - 4, + 11, ); }); diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts index f3aed366004b..f0d353323999 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts @@ -193,8 +193,7 @@ export const filterTransactions = ( apps.includes(app) ); } else { - // We don't want to show internal transactions by default. - return !isInternal; + return true; } }) .filter(