Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sql: transaction statistics aren't being cleared periodically #54452

Closed
arulajmani opened this issue Sep 16, 2020 · 0 comments · Fixed by #54453
Closed

sql: transaction statistics aren't being cleared periodically #54452

arulajmani opened this issue Sep 16, 2020 · 0 comments · Fixed by #54453
Assignees
@arulajmani
Labels
A-sql-observability Related to observability of the SQL layer C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@arulajmani
Copy link
Collaborator

Currently, when app statistics are cleared, we don't clear the whole map. Instead, we specifically clear the statement statistics map by resetting it. Now that we have an analogous transaction statistics map, this should be cleared too, so that we don't grow the transaction statistics unboundedly.
@arulajmani arulajmani self-assigned this Sep 16, 2020
@arulajmani arulajmani added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Sep 16, 2020
@cockroachdb cockroachdb deleted a comment from blathers-crl bot Sep 16, 2020
@arulajmani arulajmani added the A-sql-observability Related to observability of the SQL layer label Sep 16, 2020
This was referenced Sep 16, 2020
Merged
Closed
arulajmani added a commit to arulajmani/cockroach that referenced this issue Sep 16, 2020
@arulajmani
sql: periodically clear per app transaction statistics map
ccd0a75 
For every app, we collect both statement and transaction statistics.
This datastructure is periodically reset to ensure it doesn't get
unboundedly large. This happens by manually resetting the
datastructures inside the app stats map instead of resetting the outer
map. Recently, we started collecting transaction statistics here in
addition to statement statistics, but we never added code to reset
the transaction statistics map. This patch corrects that oversight.

Fixes cockroachdb#54452

Release note (bug fix): There was a bug in transaction statistics
collection that could let the datastructure grow unboundedly large.
This is now fixed, and the resetting happens at the same cadence as
statement statistics.
@arulajmani arulajmani mentioned this issue Sep 16, 2020
Merged
craig bot pushed a commit that referenced this issue Sep 16, 2020
@mgartner @arulajmani
Merge #54425 #54453
1bffabd 
54425: sql: add telemetry for partial indexes r=RaduBerinde a=mgartner

#### sql: add telemetry for partial index creation

This commit adds telemetry counters for the creation of partial index. A
counter is also added for the failed creation of a partial inverted
index.

Release note: None

#### opt: add telemetry for partial index usage

This commit adds telemetry counters for the usage of a partial index,
either in a scan or a lookup join.

Release note: None


54453: sql: periodically clear per app transaction statistics map r=otan a=arulajmani

For every app, we collect both statement and transaction statistics.
This datastructure is periodically reset to ensure it doesn't get
unboundedly large. This happens by manually resetting the
datastructures inside the app stats map instead of resetting the outer
map. Recently, we started collecting transaction statistics here in
addition to statement statistics, but we never added code to reset
the transaction statistics map. This patch corrects that oversight.

Fixes #54452

Release note (bug fix): There was a bug in transaction statistics
collection that could let the datastructure grow unboundedly large.
This is now fixed, and the resetting happens at the same cadence as
statement statistics.

Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: arulajmani <[email protected]>
@craig craig bot closed this as completed in 668de6b Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@arulajmani arulajmani

Labels
A-sql-observability Related to observability of the SQL layer C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql: periodically clear per app transaction statistics map arulajmani/cockroach
1 participant
@arulajmani