-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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-24.3: sql: do not collect histograms for non-indexed JSON columns #139897
Conversation
8b1e538
to
bfce5f6
Compare
Thanks for opening a backport. Please check the backport criteria before merging:
If your backport adds new functionality, please ensure that the following additional criteria are satisfied:
Also, please add a brief release justification to the body of your PR to justify this |
bfce5f6
to
1611c96
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @blathers-crl[bot] and @DrewKimball)
pkg/sql/logictest/testdata/logic_test/distsql_stats
line 1550 at r1 (raw file):
s {a} 3 0 true s {b} 3 0 true s {j} 3 0 false
nit: I'm assuming these tests should have no change given the default value true
.
docs/generated/settings/settings.html
line 316 at r1 (raw file):
<tr><td><div id="setting-sql-stats-multi-column-collection-enabled" class="anchored"><code>sql.stats.multi_column_collection.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>multi-column statistics collection mode</td><td>Serverless/Dedicated/Self-Hosted</td></tr> <tr><td><div id="setting-sql-stats-non-default-columns-min-retention-period" class="anchored"><code>sql.stats.non_default_columns.min_retention_period</code></div></td><td>duration</td><td><code>24h0m0s</code></td><td>minimum retention period for table statistics collected on non-default columns</td><td>Serverless/Dedicated/Self-Hosted</td></tr> <tr><td><div id="setting-sql-stats-non-indexed-json-histograms-enabled" class="anchored"><code>sql.stats.non_indexed_json_histograms.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>set to true to collect table statistics histograms on non-indexed JSON columns</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
nit: I think the default value needs to be updated to true
in docs.
1611c96
to
161b5ec
Compare
Informs #139381 Release note (sql change): Since v23.2 table statistics histograms have been collected for non-indexed JSON columns. Histograms are no longer collected for these columns if `sql.stats.non_indexed_json_histograms.enabled` is set to `false`. This reduces memory usage during table statistics collection, for both automatic and manual collection via `ANALYZE` and `CREATE STATISTICS`.
161b5ec
to
5eac908
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball and @yuzefovich)
docs/generated/settings/settings.html
line 316 at r1 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
nit: I think the default value needs to be updated to
true
in docs.
Done.
pkg/sql/logictest/testdata/logic_test/distsql_stats
line 1550 at r1 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
nit: I'm assuming these tests should have no change given the default value
true
.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r2, 3 of 3 files at r3, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball)
blathers backport 24.3.4-rc |
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error setting assignees, but backport branch blathers/backport-release-24.3.4-rc-139897 is ready: POST https://api.github.com/repos/cockroachdb/cockroach/issues/140265/assignees: 404 Not Found [] Backport to branch 24.3.4-rc failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Backport 1/1 commits from #139766 on behalf of @mgartner.
/cc @cockroachdb/release
Informs #139381
Release note (sql change): Since v23.2 table statistics histograms have
been collected for non-indexed JSON columns. Histograms are no longer
collected for these columns if
sql.stats.non_indexed_json_histograms.enabled
is set to
false
. This reduces memory usage during tablestatistics collection, for both automatic and manual collection via
ANALYZE
andCREATE STATISTICS
.Release justification: New cluster setting disabled by default.