Skip to content

Commit

Permalink
sqlstats: fix counter for in-memory fingerprints
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
j82w committed Oct 2, 2023
1 parent 52e085a commit cf634c5
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
14 changes: 14 additions & 0 deletions pkg/sql/sqlstats/sslocal/sql_stats_test.go
Expand Up @@ -506,6 +506,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) {
Expand Down Expand Up @@ -634,6 +645,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())
})
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/sqlstats/ssmemstorage/ss_mem_counter.go
Expand Up @@ -165,8 +165,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
Expand Down

0 comments on commit cf634c5

Please sign in to comment.