vtctl ApplySchema -uuid_list #9325
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
… their own migration UUID and at their own risk if the UUID they provide is not unique Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
shlomi-noach
added
Type: Enhancement
shlomi-noach
requested review from
ajm188,
doeg,
frouioui,
harshit-gangal and
systay
as code owners
|
@shlomi-noach looks great to me from a consumer perspective! |
mavenraven
approved these changes
Dec 7, 2021
deepthi
reviewed
Dec 8, 2021
doc/releasenotes/13_0_0_summary.md
Outdated
| @@ -60,6 +60,17 @@ For example: | |||
| vtctlclient OnlineDDL commerce complete d08ffe6b_51c9_11ec_9cf2_0a43f95f28a3 | |||
| ``` | |||
|
|
|||
| ### vtctl/vtctlclient ApplySchema -uuid | |||
There was a problem hiding this comment.
Intuitively uuid seems to indicate that it is a single uuid versus a list or a set. uuids feels awkward, but maybe uuid_list??
| @@ -245,15 +245,16 @@ func TestSchemaChange(t *testing.T) { | |||
| assert.Equal(t, 2, len(shard.Vttablets)) | |||
| } | |||
|
|
|||
| explicitUUID := "" | |||
There was a problem hiding this comment.
This is a nit, but maybe call this providedUUID? Or providedUUIDList if you accept the first suggestion :)
go/vt/schema/online_ddl.go
Outdated
| @@ -181,15 +181,18 @@ func onlineDDLStatementSanity(sql string, ddlStmt sqlparser.DDLStatement) error | |||
| } | |||
|
|
|||
| // NewOnlineDDLs takes a single DDL statement, normalizes it (potentially break down into multiple statements), and generates one or more OnlineDDL instances, one for each normalized statement | |||
| func NewOnlineDDLs(keyspace string, sql string, ddlStmt sqlparser.DDLStatement, ddlStrategySetting *DDLStrategySetting, requestContext string) (onlineDDLs [](*OnlineDDL), err error) { | |||
| func NewOnlineDDLs(keyspace string, sql string, ddlStmt sqlparser.DDLStatement, ddlStrategySetting *DDLStrategySetting, requestContext string, explicitUUID string) (onlineDDLs [](*OnlineDDL), err error) { | |||
go/vt/vtctl/vtctl.go
Outdated
| @@ -3232,6 +3232,7 @@ func commandApplySchema(ctx context.Context, wr *wrangler.Wrangler, subFlags *fl | |||
| sql := subFlags.String("sql", "", "A list of semicolon-delimited SQL commands") | |||
| sqlFile := subFlags.String("sql-file", "", "Identifies the file that contains the SQL commands") | |||
| ddlStrategy := subFlags.String("ddl_strategy", string(schema.DDLStrategyDirect), "Online DDL strategy, compatible with @@ddl_strategy session variable (examples: 'gh-ost', 'pt-osc', 'gh-ost --max-load=Threads_running=100'") | |||
| uuids := subFlags.String("uuid", "", "Optional: comma delimited explicit UUIDs for migration. If given, must match number of DDL changes") | |||
There was a problem hiding this comment.
Intuitively uuid seems to indicate that it is a single uuid versus a list or a set. uuids feels awkward for a flag name, but maybe uuid_list??
| @@ -251,21 +251,21 @@ func (t TupleExpr) Evaluate(env *ExpressionEnv) (EvalResult, error) { | |||
| } | |||
|
|
|||
| // Evaluate implements the Expr interface | |||
| func (b *BindVariable) Evaluate(env *ExpressionEnv) (EvalResult, error) { | |||
| val, ok := env.BindVars[b.Key] | |||
| func (bv *BindVariable) Evaluate(env *ExpressionEnv) (EvalResult, error) { | |||
There was a problem hiding this comment.
Changes to this file seem unrelated. Are they really required?
There was a problem hiding this comment.
Oh this was due to the new linter throwing errors in that file. I'll see what I can do here.
There was a problem hiding this comment.
ok this at least was already merged by another PR
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
|
flag
|
deepthi
approved these changes
Dec 9, 2021
3 tasks
3 tasks
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.
Description
vtctlient ApplySchemanow support a new optional-uuid_listflag. It is possible for the user to explicitly specify the UUID(s) for given migration(s). UUIDs must be in a specific format. If given, number of UUIDs must match the number of DDL statements. Example:Format is comma delimited, zero or more UUIDs.
Vitess will assign each migration with given UUID in order of appearance.
It is the user's responsibility to ensure given UUIDs are globally unique. If the user submits a migration with an already existing UUID, that migration never gets scheduled nor executed.
The purpose of this change is to give users better control over submitted migrations. Moreover, it is a means for users to achiever idempotency: the user can choose to resubmit a migration with same UUID, intentionally ensuring it does not execute again.
-uuid_listis optional. If omitted or if empty then Vitess auto-generates UUIDs for the migrations.Related Issue(s)
Checklist