From a57aa7c8f42c254b94feb36a0383419c41df2384 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 e1a64ece7f7a..4f7e4299fdc2 100644 --- a/pkg/ccl/serverccl/statusccl/tenant_status_test.go +++ b/pkg/ccl/serverccl/statusccl/tenant_status_test.go @@ -439,6 +439,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). @@ -463,7 +464,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) @@ -771,6 +772,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) @@ -789,7 +791,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 7c7507d83f39..4f8b46a3768c 100644 --- a/pkg/server/index_usage_stats_test.go +++ b/pkg/server/index_usage_stats_test.go @@ -76,6 +76,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() @@ -111,6 +116,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 @@ -150,7 +160,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) @@ -164,13 +174,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) @@ -178,6 +194,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) @@ -197,6 +216,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 @@ -204,7 +225,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") @@ -232,7 +253,7 @@ 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) } func TestGetTableID(t *testing.T) { diff --git a/pkg/sql/opt_exec_factory.go b/pkg/sql/opt_exec_factory.go index a65b2dd63f9a..77420dd3580d 100644 --- a/pkg/sql/opt_exec_factory.go +++ b/pkg/sql/opt_exec_factory.go @@ -610,9 +610,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,