release-2.1: table caching and zone config improvements #30173
Merged
Conversation
This file contains 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
Release note: None
This helper function resolves and returns a mutable table descriptor from the store. Release note: None
This helper returns an uncached database descriptor that can be used by a schema change. Schema changes on tables and other schema elements always read the database from the store even when the database itself is not going to change. Use this for an immutable database. Release note: None
It helps resolve an uncached immutable table descriptor needed in various SHOW commands. Release note: None
writeSchemaChange() already calls ValidateTable() within writeTableDescToBatch(). Release note: None
change the implementation to always return an uncached database descriptor from the store. Release note: None
the new method also resolves the index name by going to the store. It also returns a MutableTableDescriptor. Release note: None
Release note: None
Release note: None
Release note: None
This is part 1/3 of the general project to improve the UX of zone configurations in CockroachDB 2.1. This patch does the following: - it removes the EXPERIMENTAL keyword from the grammar for statements that view or change zone configurations. - it adds missing SQL contextual help strings. - it removes the "skip doc" parser annotation so that syntax diagrams can be generated. - it modifies the `cockroach zone` CLI utility to avoid using the EXPERIMENTAL keyword. For compatibility with pre-2.1 nodes, the code falls back to using EXPERIMENTAL if the syntax without EXPERIMENTAL is not recognized. Release note (cli change): `cockroach zone rm` will now produce no output unless an error occurs.
This is part 2/3 of of the general project to improve the UX of zone configurations in CockroachDB 2.1. This patch does the following: - it creates a new column `config_sql` in `crdb_internal.zones` and the output of SHOW ZONE CONFIGURATION. This contains a copy of the allowed ALTER syntax which will be introduced in the next patch. - it cleans up and simplifies the implementation ALTER ... CONFIGURE ZONE, thereby simplifying further improvements. This patch also makes ALTER ... CONFIGURE ZONE able to accept strings that do not contain a terminating newline. This makes it possible to specify unescaped strings, for example `ALTER ... CONFIGURE ZONE 'num_replicas: 3'` where previously one would have needed to specify `e'num_replicas:3 \n'`. The behavior for deletion is preserved: a NULL values requests a removal of the zone configuration; an empty string (or containing only whitespace) is a no-op (not a deletion). Release note: None
This is part 3/3 of the general project to improve the UX of zone
configurations in CockroachDB 2.1.
This patch introduces the extended syntax for the ALTER ... CONFIGURE
ZONE statement. The following forms are now supported:
```
-- New: remove a zone configuration.
ALTER ... CONFIGURE ZONE DISCARD;
-- New: reset a zone config to the inherited defaults.
ALTER ... CONFIGURE ZONE USING DEFAULT;
-- New: configure parameters using SQL expressions.
-- Setting `constraints` and `lease_preferences` still uses YAML.
ALTER ... CONFIGURE ZONE USING
num_replicas = 1, gc.ttlseconds = 3600,
constraints = '[+attr=ssd]';
-- Preserved for backward compatibility:
-- set multiple variables at once using YAML.
ALTER ... CONFIGURE ZONE = '...yaml...';
-- Preserved for backward compatibility;
-- equivalent to ALTER ... CONFIGURE ZONE DISCARD:
ALTER ... CONFIGURE ZONE = NULL;
```
The disambiguation using either `=` (or, equivalently, TO) or USING is
necessary to disambiguate the grammar (`ZONE foo = bar` would be
parsable as either the first form with expression `(foo = bar)` or the
second form with a single parameter set).
Additionally, this patch ensures (and tests) that the ALTER
... CONFIGURE ZONE statements can be prepared and use placeholders.
Release note (sql change): CockroachDB now recognizes `ALTER
... CONFIGURE ZONE` to add, modify, reset and remove zone
configurations.
Release note (sql change): The SQL statements to modify zone
configurations can now use placeholders (`$1`, etc) and be prepared
for multiple executions.
Requested/suggested by @benesch: this patch makes the various `cockroach zone` sub-commands report a deprecation warning and a reference to the new SQL interface: ``` $ cockroach zone ls Command "ls" is deprecated, use SHOW ZONE CONFIGURATIONS in a SQL client instead. $ cockroach zone get .default Command "get" is deprecated, use SHOW ZONE CONFIGURATION FOR ... in a SQL client instead. $ cockroach zone set .default Command "set" is deprecated, use ALTER ... CONFIGURE ZONE in a SQL client instead. $ cockroach zone rm liveness Command "rm" is deprecated, use ALTER ... CONFIGURE ZONE DISCARD in a SQL client instead. ``` Release note (cli change): The various `cockroach zone` sub-commands are now deprecated and will be removed in a future version of CockroachDB. Clients should use the SQL interface instead via `SHOW ZONE CONFIGURATION(s)` or `ALTER ... CONFIGURE ZONE`.
|
This change is |
|
LGTM but I am getting the impression the review process didn't go as intended? |
|
I reviewed it and hit the merge button. I suppose we're not used to that, but why not on these release branches. |
|
Definitely OK to use the big green button on these release branches! (Announcements have been made to that effect on #engineering.) I think @knz may have been expecting to review the backport of his commits before the PR merged—but no harm done, since it looked good to everyone! |
|
Oh no I was OOO so wasn't expecting to review. I was simply surprised, it's not often that the reviewer is also the person who merges. |
|
Oh, I see. In this case I opened the PR at Vivek's request so it wasn't really my PR. :) |
|
All good 👍 |
This was referenced Sep 25, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Backport:
Please see individual PRs for details. Grouping these mostly unrelated PRs together avoids a merge conflict in pkg/sql/set_zone_config.go.
/cc @cockroachdb/release
Fix #30045.