From 73f3d8c551eafb6d52084da496456bbb174fd4ac Mon Sep 17 00:00:00 2001 From: j82w Date: Mon, 2 Oct 2023 16:30:41 -0400 Subject: [PATCH] sqlstats: fix counter for in-memory fingerprints Problem: The counters used to track the number of unique fingerprints we store in-memory for sql stats were refactored in #110805. In change #110805 a bug was introduced where it incresease the memory instead of resetting the counts. This causes the statstics to stop calculating new stats once the limit is hit. Solution: Fix the bug by resetting the counters instead of increasing them. Added new test to test the reset functionality. Fixes: #111583 Release note (sql change): Fix a bug that causes the sql stats to stop collecting new stats. --- pkg/sql/sqlstats/sslocal/sql_stats_test.go | 14 ++++++++++++++ pkg/sql/sqlstats/ssmemstorage/ss_mem_counter.go | 4 ++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/pkg/sql/sqlstats/sslocal/sql_stats_test.go b/pkg/sql/sqlstats/sslocal/sql_stats_test.go index 9652c93181f4..f8ffdac7cca9 100644 --- a/pkg/sql/sqlstats/sslocal/sql_stats_test.go +++ b/pkg/sql/sqlstats/sslocal/sql_stats_test.go @@ -499,6 +499,17 @@ func TestExplicitTxnFingerprintAccounting(t *testing.T) { require.Equal(t, tc.curFingerprintCount, sqlStats.GetTotalFingerprintCount(), "testCase: %+v", tc) } + + // Verify reset works correctly. + require.NoError(t, sqlStats.Reset(ctx)) + require.Zero(t, sqlStats.GetTotalFingerprintCount()) + + // Verify the count again after the reset. + for _, tc := range testCases { + recordStats(&tc) + require.Equal(t, tc.curFingerprintCount, sqlStats.GetTotalFingerprintCount(), + "testCase: %+v", tc) + } } func TestAssociatingStmtStatsWithTxnFingerprint(t *testing.T) { @@ -627,6 +638,9 @@ func TestAssociatingStmtStatsWithTxnFingerprint(t *testing.T) { } require.Equal(t, expectedCount, len(stats), "testCase: %+v, stats: %+v", txn, stats) } + + require.NoError(t, sqlStats.Reset(ctx)) + require.Zero(t, sqlStats.GetTotalFingerprintCount()) }) } diff --git a/pkg/sql/sqlstats/ssmemstorage/ss_mem_counter.go b/pkg/sql/sqlstats/ssmemstorage/ss_mem_counter.go index 18918058f158..3c1e5f898df8 100644 --- a/pkg/sql/sqlstats/ssmemstorage/ss_mem_counter.go +++ b/pkg/sql/sqlstats/ssmemstorage/ss_mem_counter.go @@ -166,8 +166,8 @@ func (s *SQLStatsAtomicCounters) tryAddTxnFingerprint() (ok bool) { func (s *SQLStatsAtomicCounters) freeByCnt( uniqueStmtFingerprintCount, uniqueTxnFingerprintCount int64, ) { - atomic.AddInt64(&s.uniqueStmtFingerprintCount, uniqueStmtFingerprintCount) - atomic.AddInt64(&s.uniqueTxnFingerprintCount, uniqueTxnFingerprintCount) + atomic.AddInt64(&s.uniqueStmtFingerprintCount, -uniqueStmtFingerprintCount) + atomic.AddInt64(&s.uniqueTxnFingerprintCount, -uniqueTxnFingerprintCount) } // GetTotalFingerprintCount returns total number of unique statement and