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 cockroachdb#110805. In change cockroachdb#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: cockroachdb#111583

Release note (sql change): Fix a bug that causes the sql stats to stop
collecting new stats.
  • Loading branch information
j82w authored and THardy98 committed Oct 4, 2023
1 parent ef4b61b commit 73f3d8c
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 @@ -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) {
Expand Down Expand Up @@ -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())
})
}

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

0 comments on commit 73f3d8c

Please sign in to comment.