Skip to content

Change local_metadata.value to MEDIUMBLOB#5380

Merged
deepthi merged 1 commit intovitessio:masterfrom
planetscale:ds-repl-pos-len
Oct 31, 2019
Merged

Change local_metadata.value to MEDIUMBLOB#5380
deepthi merged 1 commit intovitessio:masterfrom
planetscale:ds-repl-pos-len

Conversation

@deepthi
Copy link
Collaborator

@deepthi deepthi commented Oct 30, 2019

To accomodate long replication positions, change type of column.
Fixes #5377

Signed-off-by: deepthi deepthi@planetscale.com

@deepthi deepthi requested a review from sougou as a code owner October 30, 2019 19:33
@deepthi deepthi requested review from enisoc and morgo October 30, 2019 19:34
Copy link
Member

Choose a reason for hiding this comment

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

If local_metadata fails, it could cause integrations like Orchestrator to exhibit unexpected behavior. Did we ever have PopulateMetadataTables optional like this before? Or was it always required?

Copy link
Collaborator Author

@deepthi deepthi Oct 30, 2019

Choose a reason for hiding this comment

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

That is a valid point. Which is worse, the tablet not coming up after restore or providing the wrong metadata to orchestrator?
For instance, a failure to populate metadata might mean that the newly restored tablet advertises the same alias as an existing running tablet, and that would surely confuse orchestrator.

As far as history goes, from the time it was introduced, an error populating metadata has always been fatal.

Keep _vt.local_metadata table across backup-restore operations.

So far local_metadata contained only data needed for Orchestrator and we didn't
need to preserve that data across restores because it was about the vttablet
running near MySQL. With the plan to store some data about the database itself
(e.g. id of a schema swap already executed on the database) we need to save
the table's contents in a backup and preserve it after restore.
This change achieves that. The data used by Orchestrator gets repopulated
immediately after restore, so that functionality is not changed.

f8628fc#diff-2aff8a84d6daf7d161e983b974993c7c

Copy link
Collaborator Author

@deepthi deepthi Oct 30, 2019

Choose a reason for hiding this comment

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

I have reverted this change. If we ever decide to do it, it can be a separate PR.

Copy link
Member

Choose a reason for hiding this comment

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

If this is run again, does it result in the same ERDupFieldName error that we already ignore?

Also, do we need to add NOT NULL to match the CREATE TABLE version?

Copy link
Collaborator Author

@deepthi deepthi Oct 30, 2019

Choose a reason for hiding this comment

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

Running again produces no errors.
We don't need to add NOT NULL. Changing the column type doesn't change its nullable status.
You are correct about NOT NULL. I will make the change.

…ion positions.

Signed-off-by: deepthi <deepthi@planetscale.com>
Copy link
Member

@enisoc enisoc left a comment

Choose a reason for hiding this comment

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

LGTM

@deepthi
Copy link
Collaborator Author

deepthi commented Oct 30, 2019

Tested manually that the ALTER works correctly.
Test procedure:

git checkout master
make
cd examples/local
./101_initial_cluster.sh
./lvtctl.sh BackupShard commerce/0
mysql -S ~/go/vtdataroot/vt_0000000100/mysql.sock -uroot
mysql> use _vt;
mysql> describe local_metadata;
+---------+----------------+------+-----+---------+-------+
| Field   | Type           | Null | Key | Default | Extra |
+---------+----------------+------+-----+---------+-------+
| name    | varchar(255)   | NO   | PRI | NULL    |       |
| value   | varchar(255)   | NO   |     | NULL    |       |
| db_name | varbinary(255) | NO   | PRI | NULL    |       |
+---------+----------------+------+-----+---------+-------+
3 rows in set (0.00 sec)

cd ../..
git checkout ds-repl-pos-len
make
cd examples/local
CELL=zone1 KEYSPACE=commerce UID_BASE=500 ./vttablet-up.sh 
mysql -S ~/go/vtdataroot/vt_0000000500/mysql.sock -uroot
mysql> use _vt;
mysql> describe local_metadata;
+---------+----------------+------+-----+---------+-------+
| Field   | Type           | Null | Key | Default | Extra |
+---------+----------------+------+-----+---------+-------+
| name    | varchar(255)   | NO   | PRI | NULL    |       |
| value   | mediumblob     | NO   |     | NULL    |       |
| db_name | varbinary(255) | NO   | PRI | NULL    |       |
+---------+----------------+------+-----+---------+-------+
3 rows in set (0.00 sec)
mysql> select * from local_metadata;
+--------------------+--------------------------------------------------+-------------+
| name               | value                                            | db_name     |
+--------------------+--------------------------------------------------+-------------+
| Alias              | zone1-0000000500                                 | vt_commerce |
| ClusterAlias       | commerce.0                                       | vt_commerce |
| DataCenter         | zone1                                            | vt_commerce |
| PromotionRule      | neutral                                          | vt_commerce |
| RestoredBackupTime | 2019-10-30T22:00:49Z                             | vt_commerce |
| RestorePosition    | MySQL56/a5dc5cbb-fb60-11e9-84b6-34e12d1e6711:1-9 | vt_commerce |
+--------------------+--------------------------------------------------+-------------+
6 rows in set (0.00 sec)

@deepthi deepthi merged commit 0603e64 into vitessio:master Oct 31, 2019
@deepthi deepthi deleted the ds-repl-pos-len branch October 31, 2019 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Snapshot Keyspaces: RestorePosition doesn't fit in local_metadata table

2 participants