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.

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

release-21.1: sql: set default sample_rate to 0.1 #61815

Merged
merged 2 commits into from
Mar 19, 2021

Conversation

yuzefovich
Copy link
Member

@yuzefovich yuzefovich commented Mar 10, 2021

Backport 2/2 commits from #61760.

/cc @cockroachdb/release


sql: do not reset planner if PREPARE is executed as a stmt

Previously, when executing a PREPARE command, we would always reset the
planner. This is not necessary when the origin of the command is SQL
because in that case we properly reset the planner before starting the
statement's execution. This change fixes a problem when PREPARE
statements are chosen to be sampled for execution statistics (without
the fix, we would encounter a nil pointer).

Release note: None

sql: set default sample_rate to 0.1

Fixes: #59379.

Release note (sql change): Default value for sql.txn_stats.sample_rate
cluster setting has been increased from 0 to 0.1. This means that from
now on every statement has 10% probability of being sampled for the
purposes of execution statistics. Note that no other criteria for
sampling (like the query latencies) are currently being utilized to
decide whether to sample a statement or not.

@yuzefovich yuzefovich requested a review from asubiotto March 10, 2021 22:48
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@asubiotto
Copy link
Contributor

I think we'll have to wait for the test fix (almost equal) and race fix (on the stopwatch) before merging this.

@yuzefovich yuzefovich added the do-not-merge bors won't merge a PR with this label. label Mar 11, 2021
@yuzefovich yuzefovich added do-not-merge bors won't merge a PR with this label. and removed do-not-merge bors won't merge a PR with this label. labels Mar 15, 2021
@yuzefovich
Copy link
Member Author

Ok, I think we can now proceed with this change once all the preliminary backports merge. I've pushed all of the cherry-picked commits here just to see a happy CI and will keep only the last 2 commits in this PR.

@asubiotto
Copy link
Contributor

asubiotto commented Mar 16, 2021

I want to let this bake on master one more day.

@yuzefovich
Copy link
Member Author

Sounds good

Previously, when executing a PREPARE command, we would always reset the
planner. This is not necessary when the origin of the command is SQL
because in that case we properly reset the planner before starting the
statement's execution. This change fixes a problem when PREPARE
statements are chosen to be sampled for execution statistics (without
the fix, we would encounter a nil pointer).

Release note: None
Release note (sql change): Default value for `sql.txn_stats.sample_rate`
cluster setting has been increased from 0 to 0.1. This means that from
now on every statement has 10% probability of being sampled for the
purposes of execution statistics. Note that no other criteria for
sampling (like the query latencies) are currently being utilized to
decide whether to sample a statement or not.
@yuzefovich yuzefovich removed the do-not-merge bors won't merge a PR with this label. label Mar 19, 2021
@yuzefovich
Copy link
Member Author

I don't think we've seen any fallout, so let's merge this.

@yuzefovich yuzefovich merged commit 8cdd17f into cockroachdb:release-21.1 Mar 19, 2021
@yuzefovich yuzefovich deleted the backport21.1-61760 branch March 19, 2021 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants