VReplication: Defer Secondary Index Creation

[mattlord]

Description

In this PR we add a new experimental --defer-secondary-keys flag for the MoveTables, Reshard, and Migrate vtctl client commands. When this flag is specified the target table has all secondary indexes removed from its schema prior to copying rows from the source and the indexes are re-added to the schema at the end of the copy phase.

Note: All secondary keys are re-added in a single ALTER TABLE statement and this an an online/in-place DDL operation. BUT, we execute this ALTER TABLE synchronously at this time to ensure the table is in its final expected state before we complete the copy phase for the table and move forward with the workflow. This is important as it means the source will need to retain binary logs for a period beyond how long this statement takes to complete (see docs) as there will not be another fast forward and catch-up cycle while it runs. Future TODO: try and figure out a safe and robust way to proceed with the workflow (copying next table or entering the running phase) while the ALTER TABLE is running while ensuring that all ALTERs are successfully applied before the workflow can proceed beyond the running phase (SwitchTraffic).

Note: Foreign keys are not supported generally with VReplication today. Fulltext keys are NOT deferred as they require a full table rebuild to add an internal FTS_DOC_ID field to each record and you cannot add/drop more than 1 of them per ALTER statement.

For the WHY, please see the feature request.

Manual Test and Demonstration:

Click here for details

pushd examples/local
./101_initial_cluster.sh; ./201_customer_tablets.sh;
echo

vtctldclient InitShardPrimary --force commerce/0 zone1-100
echo

command mysql -u root --socket=/opt/vtdataroot/vt_0000000100/mysql.sock  vt_commerce -e "create table sit (id int not null primary key, c1 varchar(100), c2 varchar(200), c3 varchar(200), unique key (c1,c2,c3), key (c1), key (c2))"
command mysql -u root --socket=/opt/vtdataroot/vt_0000000100/mysql.sock  vt_commerce -e "create table sit2 (id int not null, c1 varchar(100), c2 varchar(200), c3 varchar(200), unique key (c1,c2,c3), key (c1), key (c2), primary key (id, c1))"
echo

vtctlclient MoveTables -- --source commerce --all --defer-secondary-keys Create customer.commerce2customer
echo

command mysql -u root --socket=/opt/vtdataroot/vt_0000000200/mysql.sock  vt_customer -e "show create table sit\G show create table sit2\G"
echo
command mysql -v -u root --socket=/opt/vtdataroot/vt_0000000200/mysql.sock  vt_customer --binary-as-hex=false -e "select * from _vt.post_copy_action"
echo

sleep 5
command mysql -u root --socket=/opt/vtdataroot/vt_0000000200/mysql.sock  vt_customer -e "show create table sit\G show create table sit2\G"
echo
command mysql -v -u root --socket=/opt/vtdataroot/vt_0000000200/mysql.sock  vt_customer --binary-as-hex=false -e "select * from _vt.post_copy_action"
echo

vtctlclient workflow customer.commerce2customer show
echo

./401_teardown.sh
popd

Final results:

*************************** 1. row ***************************
       Table: sit
Create Table: CREATE TABLE `sit` (
  `id` int NOT NULL,
  `c1` varchar(100) DEFAULT NULL,
  `c2` varchar(200) DEFAULT NULL,
  `c3` varchar(200) DEFAULT NULL,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci
*************************** 1. row ***************************
       Table: sit2
Create Table: CREATE TABLE `sit2` (
  `id` int NOT NULL,
  `c1` varchar(100) NOT NULL,
  `c2` varchar(200) DEFAULT NULL,
  `c3` varchar(200) DEFAULT NULL,
  PRIMARY KEY (`id`,`c1`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci

--------------
select * from _vt.post_copy_action
--------------

+----+----------+------------+------------------------------------------------------------------------------------------------------------------------+
| id | vrepl_id | table_name | action                                                                                                                 |
+----+----------+------------+------------------------------------------------------------------------------------------------------------------------+
|  1 |        1 | sit2       | {"task": "alter table vt_customer.sit2 add UNIQUE KEY c1 (c1, c2, c3), add KEY c1_2 (c1), add KEY c2 (c2)", "type": 1} |
|  2 |        1 | sit        | {"task": "alter table vt_customer.sit add UNIQUE KEY c1 (c1, c2, c3), add KEY c1_2 (c1), add KEY c2 (c2)", "type": 1}  |
+----+----------+------------+------------------------------------------------------------------------------------------------------------------------+

*************************** 1. row ***************************
       Table: sit
Create Table: CREATE TABLE `sit` (
  `id` int NOT NULL,
  `c1` varchar(100) DEFAULT NULL,
  `c2` varchar(200) DEFAULT NULL,
  `c3` varchar(200) DEFAULT NULL,
  PRIMARY KEY (`id`),
  UNIQUE KEY `c1` (`c1`,`c2`,`c3`),
  KEY `c1_2` (`c1`),
  KEY `c2` (`c2`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci
*************************** 1. row ***************************
       Table: sit2
Create Table: CREATE TABLE `sit2` (
  `id` int NOT NULL,
  `c1` varchar(100) NOT NULL,
  `c2` varchar(200) DEFAULT NULL,
  `c3` varchar(200) DEFAULT NULL,
  PRIMARY KEY (`id`,`c1`),
  UNIQUE KEY `c1` (`c1`,`c2`,`c3`),
  KEY `c1_2` (`c1`),
  KEY `c2` (`c2`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci

--------------
select * from _vt.post_copy_action
--------------


{
	"Workflow": "commerce2customer",
	"SourceLocation": {
		"Keyspace": "commerce",
		"Shards": [
			"0"
		]
	},
	"TargetLocation": {
		"Keyspace": "customer",
		"Shards": [
			"0"
		]
	},
	"MaxVReplicationLag": 0,
	"MaxVReplicationTransactionLag": 0,
	"Frozen": false,
	"ShardStatuses": {
		"0/zone1-0000000201": {
			"PrimaryReplicationStatuses": [
				{
					"Shard": "0",
					"Tablet": "zone1-0000000201",
					"ID": 1,
					"Bls": {
						"keyspace": "commerce",
						"shard": "0",
						"filter": {
							"rules": [
								{
									"match": "corder",
									"filter": "select * from corder"
								},
								{
									"match": "customer",
									"filter": "select * from customer"
								},
								{
									"match": "product",
									"filter": "select * from product"
								},
								{
									"match": "sit",
									"filter": "select * from sit"
								},
								{
									"match": "sit2",
									"filter": "select * from sit2"
								}
							]
						}
					},
					"Pos": "1d1f22e8-8364-11ed-8489-8259370a8da6:1-27",
					"StopPos": "",
					"State": "Running",
					"DBName": "vt_customer",
					"TransactionTimestamp": 1671870227,
					"TimeUpdated": 1671870231,
					"TimeHeartbeat": 1671870231,
					"TimeThrottled": 0,
					"ComponentThrottled": "",
					"Message": "",
					"Tags": "",
					"WorkflowType": "MoveTables",
					"WorkflowSubType": "None",
					"CopyState": null
				}
			],
			"TabletControls": null,
			"PrimaryIsServing": true
		}
	},
	"SourceTimeZone": "",
	"TargetTimeZone": "",
	"DeferSecondaryKeys": true
}

Related Issue(s)

• Fixes: Feature Request: VReplication Defer Secondary Index Creation #11699

Checklist

• "Backport to:" labels have been added if this change should be back-ported
• Tests were added
• Documentation: Document --defer-secondary-keys for MoveTables/Reshard website#1349

[vitess-bot]

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 test is added or modified, there should be a documentation on top of the test to explain what the expected behavior is what the test does.

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.

[rohit-nayak-ps]

Functionality looks good. Nice work!

• I guess you have manually tested the full flow for this feature. But maybe we should add a couple of secondary indexes to one of the e2e tables so that the complete flow is tested as part of the e2e tests
• Can we also log the start/end of the post-copy alters to the vreplication_log table for keeping track of how long it takes?
• The PhaseTimings stat for copy phase is now going to include time for the alter. Should we also record PhaseTimings for the post-copy alter, so that we can differentiate this from the copy insert bandwidth?

[mattlord]

Functionality looks good. Nice work!

• I guess you have manually tested the full flow for this feature. But maybe we should add a couple of secondary indexes to one of the e2e tables so that the complete flow is tested as part of the e2e tests
• Can we also log the start/end of the post-copy alters to the vreplication_log table for keeping track of how long it takes?
• The PhaseTimings stat for copy phase is now going to include time for the alter. Should we also record PhaseTimings for the post-copy alter, so that we can differentiate this from the copy insert bandwidth?

Thanks for the great suggestions, @rohit-nayak-ps! I believe that I addressed all of them here: 5879ccc

Adding secondary indexes to the e2e tables and enabling --defer-secondary-keys (for v2) actually highlighted a couple of edge cases with SQL_MODE and Reshard merges/consolidations that I addressed in the commit. 🙂

[rohit-nayak-ps]

Nice work! Should significantly speed up a lot of production workflows :-)