Skip to content

Allow users to control VReplication DDL handling#11532

Merged
mattlord merged 16 commits intovitessio:mainfrom
planetscale:vrepl_expose_onddl
Nov 10, 2022
Merged

Allow users to control VReplication DDL handling#11532
mattlord merged 16 commits intovitessio:mainfrom
planetscale:vrepl_expose_onddl

Conversation

@mattlord
Copy link
Copy Markdown
Member

@mattlord mattlord commented Oct 19, 2022

Description

How VReplication handled DDL encountered in the replication stream was always technically variable and has a passing mention in the docs, but we provided no way to set it as it wasn't recommended to use anything other than the default of IGNORE (DDL events are ignored by the vapplier component on the target shards) — and instead it was recommended that for schema changes which you wanted on the target side, you execute those DDL on the target side first, then on the source side. Technically you could change it but it required modifying the prototext value in the workflow's _vt.vreplication.source field. So in practice, it did not exist.

It's been pointed out, however, that it's at least useful to specify a value of STOP (stop the workflow) so that the workflow is stopped and other orchestration tooling can take action such as Cancel'ing the workflow and starting another new one. It's also reasonable that if you really know what you're doing, you could set the handling to EXEC (execute the DDL and treat any errors as workflow errors) or even EXEC_IGNORE (execute the DDL but ignore any errors). The documentation will need to add proper warnings about the EXEC* options.

This PR does the following:

  1. Adds a --on-ddl=<action> flag for the MoveTables and Reshard commands
  2. Stores this parameter in the workflow config (_vt.vreplication.source value)
  3. Shows the workflow parameter as a top-level OnDDL key in the Workflow show command output IF you specified a non-default (0/IGNORE) value so that we have less noise in the output for most users

Manual Test

Click here for details
make build && pushd examples/local

./101_initial_cluster.sh; ./201_customer_tablets.sh

vtctlclient MoveTables -- --source commerce --tables 'customer,corder' --on-ddl=stop Create customer.commerce2customer
vtctlclient workflow customer.commerce2customer show
mysql commerce -e "alter table customer add notes varchar(100)"
vtctlclient workflow customer.commerce2customer show
vtctlclient MoveTables -- --source commerce --tables 'customer,corder' --on-ddl=stop Cancel customer.commerce2customer

# The default of IGNORE should be used, which means that:
#  1. The OnDDL field is not shown in the SHOW output
#  2. The workflow should continue running fine
vtctlclient MoveTables -- --source commerce --tables 'customer,corder' Create customer.commerce2customer
mysql commerce -e "alter table customer drop notes"
vtctlclient workflow customer.commerce2customer show

With the final results being:

{
	"Workflow": "commerce2customer",
	"SourceLocation": {
		"Keyspace": "commerce",
		"Shards": [
			"0"
		]
	},
	"TargetLocation": {
		"Keyspace": "customer",
		"Shards": [
			"0"
		]
	},
	"MaxVReplicationLag": 0,
	"MaxVReplicationTransactionLag": 0,
	"Frozen": false,
	"ShardStatuses": {
		"0/zone1-0000000200": {
			"PrimaryReplicationStatuses": [
				{
					"Shard": "0",
					"Tablet": "zone1-0000000200",
					"ID": 1,
					"Bls": {
						"keyspace": "commerce",
						"shard": "0",
						"filter": {
							"rules": [
								{
									"match": "customer",
									"filter": "select * from customer"
								},
								{
									"match": "corder",
									"filter": "select * from corder"
								}
							]
						}
					},
					"Pos": "6d3867c2-4f60-11ed-8fd8-b5d81afa9601:1-67",
					"StopPos": "",
					"State": "Stopped",
					"DBName": "vt_customer",
					"TransactionTimestamp": 1666151181,
					"TimeUpdated": 1666151181,
					"TimeHeartbeat": 1666151181,
					"TimeThrottled": 0,
					"ComponentThrottled": "",
					"Message": "Stopped at DDL alter table customer add column notes varchar(100)",
					"Tags": "",
					"WorkflowType": "MoveTables",
					"WorkflowSubType": "None",
					"CopyState": null
				}
			],
			"TabletControls": null,
			"PrimaryIsServing": true
		}
	},
	"SourceTimeZone": "",
	"TargetTimeZone": "",
	"OnDDL": "STOP"
}
...
{
	"Workflow": "commerce2customer",
	"SourceLocation": {
		"Keyspace": "commerce",
		"Shards": [
			"0"
		]
	},
	"TargetLocation": {
		"Keyspace": "customer",
		"Shards": [
			"0"
		]
	},
	"MaxVReplicationLag": 0,
	"MaxVReplicationTransactionLag": 0,
	"Frozen": false,
	"ShardStatuses": {
		"0/zone1-0000000200": {
			"PrimaryReplicationStatuses": [
				{
					"Shard": "0",
					"Tablet": "zone1-0000000200",
					"ID": 2,
					"Bls": {
						"keyspace": "commerce",
						"shard": "0",
						"filter": {
							"rules": [
								{
									"match": "customer",
									"filter": "select * from customer"
								},
								{
									"match": "corder",
									"filter": "select * from corder"
								}
							]
						}
					},
					"Pos": "6d3867c2-4f60-11ed-8fd8-b5d81afa9601:1-70",
					"StopPos": "",
					"State": "Running",
					"DBName": "vt_customer",
					"TransactionTimestamp": 1666151183,
					"TimeUpdated": 1666151183,
					"TimeHeartbeat": 1666151183,
					"TimeThrottled": 0,
					"ComponentThrottled": "",
					"Message": "",
					"Tags": "",
					"WorkflowType": "MoveTables",
					"WorkflowSubType": "None",
					"CopyState": null
				}
			],
			"TabletControls": null,
			"PrimaryIsServing": true
		}
	},
	"SourceTimeZone": "",
	"TargetTimeZone": ""
}

Related Issue(s)

Checklist

Signed-off-by: Matt Lord <mattalord@gmail.com>
@mattlord mattlord added Type: Enhancement Logical improvement (somewhere between a bug and feature) Component: VReplication labels Oct 19, 2022
@vitess-bot
Copy link
Copy Markdown
Contributor

vitess-bot bot commented Oct 19, 2022

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

  • Ensure that the Pull Request has a descriptive title.
  • If this is a change that users need to know about, please apply the release notes (needs details) label so that merging is blocked unless the summary release notes document is included.

If a new flag is being introduced:

  • Is it really necessary to add this flag?
  • Flag names should be clear and intuitive (as far as possible)
  • Help text should be descriptive.
  • Flag names should use dashes (-) as word separators rather than underscores (_).

If a workflow is added or modified:

  • Each item in Jobs should be named in order to mark it as required.
  • If the workflow should be required, the maintainer team should be notified.

Bug fixes

  • There should be at least one unit or end-to-end test.
  • The Pull Request description should include a link to an issue that describes the bug.

Non-trivial changes

  • There should be some code comments as to why things are implemented the way they are.

New/Existing features

  • Should be documented, either by modifying the existing documentation or creating new documentation.
  • New features should have a link to a feature request issue or an RFC that documents the use cases, corner cases and test cases.

Backward compatibility

  • Protobuf changes should be wire-compatible.
  • Changes to _vt tables and RPCs need to be backward compatible.
  • vtctl command output order should be stable and awk-able.
  • RPC changes should be compatible with vitess-operator
  • If a flag is removed, then it should also be removed from VTop, if used there.

Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
@mattlord mattlord force-pushed the vrepl_expose_onddl branch 2 times, most recently from 541d835 to af4f5ae Compare October 20, 2022 23:30
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
return resp, respBody, err
}

// TestVreplicationDDLHandling tests the DDL handling in
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need an end-to-end test here? We have been discussing about reducing CI load by being careful about reviewing existing e2e tests where unit tests coverage is possible.

Copy link
Copy Markdown
Member

@rohit-nayak-ps rohit-nayak-ps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functionality looks good!

We need to evaluate the need for the e2e test, otherwise lgtm.

Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Copy link
Copy Markdown
Member

@rohit-nayak-ps rohit-nayak-ps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Signed-off-by: Matt Lord <mattalord@gmail.com>
Copy link
Copy Markdown
Contributor

@ajm188 ajm188 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(just leaving the placeholder comment, which i know you're pushing a fix for) LGTM!


import (
"context"
_ "flag"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Copy Markdown
Collaborator

@deepthi deepthi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proto changes lgtm

@mattlord mattlord merged commit 1ace3b4 into vitessio:main Nov 10, 2022
@mattlord mattlord deleted the vrepl_expose_onddl branch November 10, 2022 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: VReplication Type: Enhancement Logical improvement (somewhere between a bug and feature)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants