From a19efd23eda1119132dfdfe24d39a4ad4d49ac29 Mon Sep 17 00:00:00 2001 From: Xin Hao Zhang Date: Wed, 23 Mar 2022 11:22:47 -0400 Subject: [PATCH] sql: record index usage stats in index joins Fixes #76173 Previously, the construction of index joins skipped the recording of the primary key for index usage stats. This commit records the use of the primary key if the statement is not of the EXPLAIN variety. Release note (bug fix): Index usage stats are now properly captured for index joins. --- .../serverccl/statusccl/tenant_status_test.go | 6 ++-- pkg/server/index_usage_stats_test.go | 29 ++++++++++++++++--- pkg/sql/opt_exec_factory.go | 11 ++++++- 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/pkg/ccl/serverccl/statusccl/tenant_status_test.go b/pkg/ccl/serverccl/statusccl/tenant_status_test.go index 580434d58ce4..c2d272d728d3 100644 --- a/pkg/ccl/serverccl/statusccl/tenant_status_test.go +++ b/pkg/ccl/serverccl/statusccl/tenant_status_test.go @@ -412,6 +412,7 @@ VALUES (1, 10, 100), (2, 20, 200), (3, 30, 300) Exec(t, "SELECT * FROM test") // Record scan on secondary index. + // Note that this is an index join and will also read from the primary index. cluster.tenantConn(randomServer). Exec(t, "SELECT * FROM test@test_a_idx") testTableIDStr := cluster.tenantConn(randomServer). @@ -436,7 +437,7 @@ WHERE table_id = ` + testTableIDStr // Assert index usage data was inserted. expected := [][]string{ - {testTableIDStr, "1", "1", "true"}, + {testTableIDStr, "1", "2", "true"}, // Primary index {testTableIDStr, "2", "1", "true"}, } cluster.tenantConn(randomServer).CheckQueryResults(t, query, expected) @@ -617,6 +618,7 @@ VALUES (1, 10, 100), (2, 20, 200), (3, 30, 300) testingCluster.tenantConn(0).Exec(t, "SELECT * FROM idx_test.test") // Record scan on secondary index. + // Note that this is an index join and will also read from the primary index. testingCluster.tenantConn(1).Exec(t, "SELECT * FROM idx_test.test@test_a_idx") testTableIDStr := testingCluster.tenantConn(2).QueryStr(t, "SELECT 'idx_test.test'::regclass::oid")[0][0] testTableID, err := strconv.Atoi(testTableIDStr) @@ -635,7 +637,7 @@ WHERE ` actual := testingCluster.tenantConn(2).QueryStr(t, query, testTableID) expected := [][]string{ - {testTableIDStr, "1", "1", "true"}, + {testTableIDStr, "1", "2", "true"}, {testTableIDStr, "2", "1", "true"}, } diff --git a/pkg/server/index_usage_stats_test.go b/pkg/server/index_usage_stats_test.go index b9f71d6bd0ad..9588c827e8d4 100644 --- a/pkg/server/index_usage_stats_test.go +++ b/pkg/server/index_usage_stats_test.go @@ -75,6 +75,11 @@ func TestStatusAPIIndexUsage(t *testing.T) { LastRead: timeutil.Now(), } + expectedStatsIndexPrimary := roachpb.IndexUsageStatistics{ + TotalReadCount: 1, + LastRead: timeutil.Now(), + } + firstPgURL, firstServerConnCleanup := sqlutils.PGUrl( t, firstServer.ServingSQLAddr(), "CreateConnections" /* prefix */, url.User(security.RootUser)) defer firstServerConnCleanup() @@ -110,6 +115,11 @@ func TestStatusAPIIndexUsage(t *testing.T) { require.NoError(t, err) require.False(t, rows.Next()) + indexKeyPrimary := roachpb.IndexUsageKey{ + TableID: roachpb.TableID(tableID), + IndexID: 1, // t@t_pkey + } + indexKeyA := roachpb.IndexUsageKey{ TableID: roachpb.TableID(tableID), IndexID: 2, // t@t_a_idx @@ -149,7 +159,7 @@ func TestStatusAPIIndexUsage(t *testing.T) { _, err = secondServerSQLConn.Exec("SELECT k FROM t WHERE a = 10 AND b = 200") require.NoError(t, err) - // Record a full scan over t_b_idx. + // Record an index join and full scan of t_b_idx. _, err = secondServerSQLConn.Exec("SELECT * FROM t@t_b_idx") require.NoError(t, err) @@ -163,13 +173,19 @@ func TestStatusAPIIndexUsage(t *testing.T) { thirdLocalStatsReader := thirdServer.SQLServer().(*sql.Server).GetLocalIndexStatistics() // First node should have nothing. - stats := firstLocalStatsReader.Get(indexKeyA.TableID, indexKeyA.IndexID) + stats := firstLocalStatsReader.Get(indexKeyPrimary.TableID, indexKeyPrimary.IndexID) + require.Equal(t, roachpb.IndexUsageStatistics{}, stats, "expecting empty stats on node 1, but found %v", stats) + + stats = firstLocalStatsReader.Get(indexKeyA.TableID, indexKeyA.IndexID) require.Equal(t, roachpb.IndexUsageStatistics{}, stats, "expecting empty stats on node 1, but found %v", stats) stats = firstLocalStatsReader.Get(indexKeyB.TableID, indexKeyB.IndexID) require.Equal(t, roachpb.IndexUsageStatistics{}, stats, "expecting empty stats on node 1, but found %v", stats) // Third node should have nothing. + stats = firstLocalStatsReader.Get(indexKeyPrimary.TableID, indexKeyPrimary.IndexID) + require.Equal(t, roachpb.IndexUsageStatistics{}, stats, "expecting empty stats on node 3, but found %v", stats) + stats = thirdLocalStatsReader.Get(indexKeyA.TableID, indexKeyA.IndexID) require.Equal(t, roachpb.IndexUsageStatistics{}, stats, "expecting empty stats on node 3, but found %v", stats) @@ -177,6 +193,9 @@ func TestStatusAPIIndexUsage(t *testing.T) { require.Equal(t, roachpb.IndexUsageStatistics{}, stats, "expecting empty stats on node 1, but found %v", stats) // Second server should have nonempty local storage. + stats = secondLocalStatsReader.Get(indexKeyPrimary.TableID, indexKeyPrimary.IndexID) + compareStatsHelper(t, expectedStatsIndexPrimary, stats, time.Minute) + stats = secondLocalStatsReader.Get(indexKeyA.TableID, indexKeyA.IndexID) compareStatsHelper(t, expectedStatsIndexA, stats, time.Minute) @@ -196,6 +215,8 @@ func TestStatusAPIIndexUsage(t *testing.T) { } statsEntries++ switch stats.Key.IndexID { + case indexKeyPrimary.IndexID: // t@t_pkey + compareStatsHelper(t, expectedStatsIndexPrimary, stats.Stats, time.Minute) case indexKeyA.IndexID: // t@t_a_idx compareStatsHelper(t, expectedStatsIndexA, stats.Stats, time.Minute) case indexKeyB.IndexID: // t@t_b_idx @@ -203,7 +224,7 @@ func TestStatusAPIIndexUsage(t *testing.T) { } } - require.True(t, statsEntries == 2, "expect to find two stats entries in RPC response, but found %d", statsEntries) + require.Equal(t, 3, statsEntries, "expect to find 3 stats entries in RPC response, but found %d", statsEntries) // Test disabling subsystem. _, err = secondServerSQLConn.Exec("SET CLUSTER SETTING sql.metrics.index_usage_stats.enabled = false") @@ -231,5 +252,5 @@ func TestStatusAPIIndexUsage(t *testing.T) { compareStatsHelper(t, expectedStatsIndexB, stats.Stats, time.Minute) } } - require.True(t, statsEntries == 2, "expect to find two stats entries in RPC response, but found %d", statsEntries) + require.Equal(t, 3, statsEntries, "expect to find 3 stats entries in RPC response, but found %d", statsEntries) } diff --git a/pkg/sql/opt_exec_factory.go b/pkg/sql/opt_exec_factory.go index 3dacf038cfd1..7ba2c8595b3d 100644 --- a/pkg/sql/opt_exec_factory.go +++ b/pkg/sql/opt_exec_factory.go @@ -612,9 +612,18 @@ func (ef *execFactory) ConstructIndexJoin( return nil, err } - tableScan.index = tabDesc.GetPrimaryIndex() + idx := tabDesc.GetPrimaryIndex() + tableScan.index = idx tableScan.disableBatchLimit() + if !ef.isExplain { + idxUsageKey := roachpb.IndexUsageKey{ + TableID: roachpb.TableID(tabDesc.GetID()), + IndexID: roachpb.IndexID(idx.GetID()), + } + ef.planner.extendedEvalCtx.indexUsageStats.RecordRead(idxUsageKey) + } + n := &indexJoinNode{ input: input.(planNode), table: tableScan,