sql: Change to using a session variable instead of FORCE syntax for overriding zone configuration settings for MR databases and tables#62008
Merged
craig[bot] merged 3 commits intocockroachdb:masterfrom Mar 16, 2021
Conversation
Member
|
This change is |
6252e76 to
f000c85
Compare
Collaborator
Author
|
Hold off on reviews. Still resolving test case failures. |
Contributor
|
from looks like it's failing doing something here: cockroach/pkg/cli/initial_sql.go Line 97 in 2d3c34a |
f000c85 to
9619033
Compare
Collaborator
Author
|
Thx. Debugger FTW. There was a null pointer exception. |
Contributor
this ain't java :P |
Collaborator
Author
|
gah! I come from the land of C. It's NULL there too.
…On Mon, Mar 15, 2021 at 6:29 PM Oliver Tan ***@***.***> wrote:
null pointer exception
this ain't java :P
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#62008 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEMXVOTBLBUS4BTMGMIFYLTTD2C3NANCNFSM4ZGWAWSA>
.
|
a46737c to
1c38b12
Compare
Collaborator
Author
|
RFAL now. Only one testcase failure remaining, which I'm hoping to resolve in the morning. |
otan
approved these changes
Mar 16, 2021
Contributor
otan
left a comment
There was a problem hiding this comment.
Reviewed 11 of 11 files at r1, 14 of 14 files at r2, 1 of 1 files at r3.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @ajstorm and @arulajmani)
pkg/ccl/multiregionccl/regional_by_row_test.go, line 550 at r2 (raw file):
testutils.SucceedsSoon(t, func() error { _, err = sqlDB.Exec(`BEGIN; SET override_multi_region_zone_config = true;
nit: indentation
a981b0d to
64c5874
Compare
Block users from updating the zone configurations of multi-region tables, without first having them set the session variable `override_multi_region_zone_config` to true. We block updates to multi-region table/partition/index zone configurations because we don't want users to accidentally override the prescribed settings, leaving them open to sub-optimal performance. Note that only the multi-region fields of the zone configuration are blocked behind this variable. All other fields (gc.ttlseconds, range_min/max_bytes, etc) can be updated without overriding. Release note (sql change): Block users from updating the zone configurations of multi-region tables.
With cockroachdb#61499 we introduced new syntax (FORCE) to override setting the zone configurations on multi-region databases. Upon further reflection, it was decided that a session variable would be better suited to the task. This commit pulls out the FORCE syntax and replaces it with the use of override_multi_region_zone_config; Release note (sql change): Revert the release notes on cockroachdb#61499. We now use a session variable (override_multi_region_zone_config) to override the zone configuration on multi-region databases (and tables).
Enable the TestIndexCleanupAfterAlterFromRegionalByRow under race. Release note: None
64c5874 to
3c2d556
Compare
Collaborator
Author
|
TFTR! bors r=otan |
Collaborator
Author
|
bors r- Just noticed the bazel failure... |
Contributor
|
Canceled. |
Collaborator
Author
|
TFTR! bors r=otan |
Contributor
|
Build succeeded: |
This was referenced Mar 16, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
sql: Block zone config updates to multi-region tables
Block users from updating the zone configurations of multi-region
tables, without first having them set the session variable
override_multi_region_zone_configto true. We block updates tomulti-region table/partition/index zone configurations because we don't
want users to accidentally override the prescribed settings, leaving
them open to sub-optimal performance. Note that only the multi-region
fields of the zone configuration are blocked behind this variable. All
other fields (gc.ttlseconds, range_min/max_bytes, etc) can be updated
without overriding.
Release note (sql change): Block users from updating the zone
configurations of multi-region tables.
sql: Use session variable instead of FORCE to override zone configs
With #61499 we introduced new syntax (FORCE) to override setting the
zone configurations on multi-region databases. Upon further reflection,
it was decided that a session variable would be better suited to the
task. This commit pulls out the FORCE syntax and replaces it with the
use of override_multi_region_zone_config;
Release note (sql change): Revert the release notes on #61499. We now
use a session variable (override_multi_region_zone_config) to override
the zone configuration on multi-region databases (and tables).
Resolves: #57668.
Note to reviewers: Please ignore the first commit, as it's being separately reviewed as part of #61889. That commit was pulle out into a separate PR as it's a release blocker, and there was a need to get it in urgently.