diff --git a/go/vt/mysqlctl/backup.go b/go/vt/mysqlctl/backup.go index e8df43cf8af..f057cdf4b12 100644 --- a/go/vt/mysqlctl/backup.go +++ b/go/vt/mysqlctl/backup.go @@ -246,6 +246,8 @@ func Restore(ctx context.Context, params RestoreParams) (*BackupManifest, error) return nil, vterrors.Wrap(err, "ListBackups failed") } + metadataManager := &MetadataManager{} + if len(bhs) == 0 { // There are no backups (not even broken/incomplete ones). params.Logger.Errorf("no backup to restore on BackupStorage for directory %v. Starting up empty.", backupDir) @@ -259,7 +261,7 @@ func Restore(ctx context.Context, params RestoreParams) (*BackupManifest, error) params.Logger.Errorf("error resetting replication: %v. Continuing", err) } - if err := PopulateMetadataTables(params.Mysqld, params.LocalMetadata, params.DbName); err != nil { + if err := metadataManager.PopulateMetadataTables(params.Mysqld, params.LocalMetadata, params.DbName); err != nil { params.Logger.Errorf("error populating metadata tables: %v. Continuing", err) } @@ -308,7 +310,7 @@ func Restore(ctx context.Context, params RestoreParams) (*BackupManifest, error) // Populate local_metadata before starting without --skip-networking, // so it's there before we start announcing ourselves. params.Logger.Infof("Restore: populating local_metadata") - err = PopulateMetadataTables(params.Mysqld, params.LocalMetadata, params.DbName) + err = metadataManager.PopulateMetadataTables(params.Mysqld, params.LocalMetadata, params.DbName) if err != nil { return nil, err } diff --git a/go/vt/mysqlctl/metadata_tables.go b/go/vt/mysqlctl/metadata_tables.go index 6b9f5413911..5ed7bb9cfe1 100644 --- a/go/vt/mysqlctl/metadata_tables.go +++ b/go/vt/mysqlctl/metadata_tables.go @@ -63,27 +63,8 @@ var ( // and _vt.shard_metadata tables. type MetadataManager struct{} -// CreateMetadataTables creates the metadata tables. See the package-level -// function for more details. -func (m *MetadataManager) CreateMetadataTables(mysqld MysqlDaemon, dbName string) error { - return CreateMetadataTables(mysqld, dbName) -} - -// PopulateMetadataTables creates and fills the metadata tables. See the -// package-level function for more details. -func (m *MetadataManager) PopulateMetadataTables(mysqld MysqlDaemon, localMetadata map[string]string, dbName string) error { - return PopulateMetadataTables(mysqld, localMetadata, dbName) -} - -// UpsertLocalMetadata adds the given metadata map to the _vt.local_metadata -// table, updating any duplicate rows to the values in the map. See the package- -// level function for more details. -func (m *MetadataManager) UpsertLocalMetadata(mysqld MysqlDaemon, localMetadata map[string]string, dbName string) error { - return UpsertLocalMetadata(mysqld, localMetadata, dbName) -} - -// CreateMetadataTables creates the _vt.local_metadata and _vt.shard_metadata -// tables. +// PopulateMetadataTables creates and fills the _vt.local_metadata table and +// creates the _vt.shard_metadata table. // // _vt.local_metadata table is a per-tablet table that is never replicated. // This allows queries against local_metadata to return different values on @@ -94,8 +75,45 @@ func (m *MetadataManager) UpsertLocalMetadata(mysqld MysqlDaemon, localMetadata // created here to make it easier to create it on databases that were running // old version of Vitess, or databases that are getting converted to run under // Vitess. -func CreateMetadataTables(mysqld MysqlDaemon, dbName string) error { - log.Infof("Creating _vt.local_metadata and _vt.shard_metadata tables ...") +// +// This function is semantically equivalent to calling createMetadataTables +// followed immediately by upsertLocalMetadata. +func (m *MetadataManager) PopulateMetadataTables(mysqld MysqlDaemon, localMetadata map[string]string, dbName string) error { + log.Infof("Populating _vt.local_metadata table...") + + // Get a non-pooled DBA connection. + conn, err := mysqld.GetDbaConnection(context.TODO()) + if err != nil { + return err + } + defer conn.Close() + + // Disable replication on this session. We close the connection after using + // it, so there's no need to re-enable replication when we're done. + if _, err := conn.ExecuteFetch("SET @@session.sql_log_bin = 0", 0, false); err != nil { + return err + } + + // Create the database and table if necessary. + if err := createMetadataTables(conn, dbName); err != nil { + return err + } + + // Populate local_metadata from the passed list of values. + return upsertLocalMetadata(conn, localMetadata, dbName) +} + +// UpsertLocalMetadata adds the given metadata map to the _vt.local_metadata +// table, updating any rows that exist for a given `_vt.local_metadata.name` +// with the map value. The session that performs these upserts sets +// sql_log_bin=0, as the _vt.local_metadata table is meant to never be +// replicated. +// +// Callers are responsible for ensuring the _vt.local_metadata table exists +// before calling this function, usually by calling CreateMetadataTables at +// least once prior. +func (m *MetadataManager) UpsertLocalMetadata(mysqld MysqlDaemon, localMetadata map[string]string, dbName string) error { + log.Infof("Upserting _vt.local_metadata ...") conn, err := mysqld.GetDbaConnection(context.TODO()) if err != nil { @@ -109,7 +127,7 @@ func CreateMetadataTables(mysqld MysqlDaemon, dbName string) error { return err } - return createMetadataTables(conn, dbName) + return upsertLocalMetadata(conn, localMetadata, dbName) } func createMetadataTables(conn *dbconnpool.DBConnection, dbName string) error { @@ -174,37 +192,7 @@ func createShardMetadataTable(conn *dbconnpool.DBConnection, dbName string) erro return nil } -// PopulateMetadataTables creates and fills the _vt.local_metadata table and -// creates _vt.shard_metadata table. -// -// This function is semantically equivalent to calling CreateMetadataTables -// followed immediately by UpsertLocalMetadata. -func PopulateMetadataTables(mysqld MysqlDaemon, localMetadata map[string]string, dbName string) error { - log.Infof("Populating _vt.local_metadata table...") - - // Get a non-pooled DBA connection. - conn, err := mysqld.GetDbaConnection(context.TODO()) - if err != nil { - return err - } - defer conn.Close() - - // Disable replication on this session. We close the connection after using - // it, so there's no need to re-enable replication when we're done. - if _, err := conn.ExecuteFetch("SET @@session.sql_log_bin = 0", 0, false); err != nil { - return err - } - - // Create the database and table if necessary. - if err := createMetadataTables(conn, dbName); err != nil { - return err - } - - // Populate local_metadata from the passed list of values. - return upsertLocalMetadata(conn, localMetadata, dbName) -} - -// UpsertLocalMetadata adds the given metadata map to the _vt.local_metadata +// upsertLocalMetadata adds the given metadata map to the _vt.local_metadata // table, updating any rows that exist for a given `_vt.local_metadata.name` // with the map value. The session that performs these upserts sets // sql_log_bin=0, as the _vt.local_metadata table is meant to never be @@ -213,24 +201,6 @@ func PopulateMetadataTables(mysqld MysqlDaemon, localMetadata map[string]string, // Callers are responsible for ensuring the _vt.local_metadata table exists // before calling this function, usually by calling CreateMetadataTables at // least once prior. -func UpsertLocalMetadata(mysqld MysqlDaemon, localMetadata map[string]string, dbName string) error { - log.Infof("Upserting _vt.local_metadata ...") - - conn, err := mysqld.GetDbaConnection(context.TODO()) - if err != nil { - return err - } - defer conn.Close() - - // Disable replication on this session. We close the connection after using - // it, so there's no need to re-enable replication when we're done. - if _, err := conn.ExecuteFetch("SET @@session.sql_log_bin = 0", 0, false); err != nil { - return err - } - - return upsertLocalMetadata(conn, localMetadata, dbName) -} - func upsertLocalMetadata(conn *dbconnpool.DBConnection, localMetadata map[string]string, dbName string) error { // Populate local_metadata from the passed list of values. if _, err := conn.ExecuteFetch("BEGIN", 0, false); err != nil { diff --git a/go/vt/vttablet/tabletmanager/restore.go b/go/vt/vttablet/tabletmanager/restore.go index f0ad6f5448c..e6df4faa881 100644 --- a/go/vt/vttablet/tabletmanager/restore.go +++ b/go/vt/vttablet/tabletmanager/restore.go @@ -184,7 +184,11 @@ func (tm *TabletManager) restoreDataLocked(ctx context.Context, logger logutil.L } if !ok { params.Logger.Infof("Attempting to restore, but mysqld already contains data. Assuming vttablet was just restarted.") - return mysqlctl.PopulateMetadataTables(params.Mysqld, params.LocalMetadata, params.DbName) + // (NOTE:@ajm188) the legacy behavior is to always populate the metadata + // tables in this branch. Since tm.MetadataManager could be nil, we + // create a new instance for use here. + metadataManager := &mysqlctl.MetadataManager{} + return metadataManager.PopulateMetadataTables(params.Mysqld, params.LocalMetadata, params.DbName) } // We should not become master after restore, because that would incorrectly // start a new master term, and it's likely our data dir will be out of date.