br: remove duplicate bind info after snapshot restore (#48946)

close #46527
pingcap · Dec 10, 2023 · e62699c · e62699c
1 parent 899dfe8
commit e62699c
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 11 deletions.
diff --git a/br/pkg/restore/BUILD.bazel b/br/pkg/restore/BUILD.bazel
@@ -48,6 +48,7 @@ go_library(
         "//br/pkg/utils/iter",
         "//br/pkg/utils/storewatch",
         "//br/pkg/version",
+        "//pkg/bindinfo",
         "//pkg/config",
         "//pkg/ddl",
         "//pkg/ddl/util",

diff --git a/br/pkg/restore/systable_restore.go b/br/pkg/restore/systable_restore.go
@@ -12,6 +12,7 @@ import (
 	berrors "github.com/pingcap/tidb/br/pkg/errors"
 	"github.com/pingcap/tidb/br/pkg/logutil"
 	"github.com/pingcap/tidb/br/pkg/utils"
+	"github.com/pingcap/tidb/pkg/bindinfo"
 	"github.com/pingcap/tidb/pkg/parser/model"
 	"github.com/pingcap/tidb/pkg/parser/mysql"
 	filter "github.com/pingcap/tidb/pkg/util/table-filter"
@@ -148,26 +149,26 @@ func (rc *Client) ClearSystemUsers(ctx context.Context, resetUsers []string) err
 
 // RestoreSystemSchemas restores the system schema(i.e. the `mysql` schema).
 // Detail see https://github.com/pingcap/br/issues/679#issuecomment-762592254.
-func (rc *Client) RestoreSystemSchemas(ctx context.Context, f filter.Filter) {
+func (rc *Client) RestoreSystemSchemas(ctx context.Context, f filter.Filter) error {
 	sysDB := mysql.SystemDB
 
 	temporaryDB := utils.TemporaryDBName(sysDB)
 	defer rc.cleanTemporaryDatabase(ctx, sysDB)
 
 	if !f.MatchSchema(sysDB) || !rc.withSysTable {
 		log.Debug("system database filtered out", zap.String("database", sysDB))
-		return
+		return nil
 	}
 	originDatabase, ok := rc.databases[temporaryDB.O]
 	if !ok {
 		log.Info("system database not backed up, skipping", zap.String("database", sysDB))
-		return
+		return nil
 	}
 	db, ok := rc.getDatabaseByName(sysDB)
 	if !ok {
 		// Or should we create the database here?
 		log.Warn("target database not exist, aborting", zap.String("database", sysDB))
-		return
+		return nil
 	}
 
 	tablesRestored := make([]string, 0, len(originDatabase.Tables))
@@ -179,15 +180,15 @@ func (rc *Client) RestoreSystemSchemas(ctx context.Context, f filter.Filter) {
 					logutil.ShortError(err),
 					zap.Stringer("table", tableName),
 				)
+				return errors.Annotatef(err, "error during merging temporary tables into system tables, table: %s", tableName)
 			}
 			tablesRestored = append(tablesRestored, tableName.L)
 		}
 	}
-	if err := rc.afterSystemTablesReplaced(tablesRestored); err != nil {
-		for _, e := range multierr.Errors(err) {
-			log.Warn("error during reconfigurating the system tables", zap.String("database", sysDB), logutil.ShortError(e))
-		}
+	if err := rc.afterSystemTablesReplaced(ctx, tablesRestored); err != nil {
+		return errors.Annotate(err, "error during extra works after system tables replaced")
 	}
+	return nil
 }
 
 // database is a record of a database.
@@ -218,20 +219,28 @@ func (rc *Client) getDatabaseByName(name string) (*database, bool) {
 
 // afterSystemTablesReplaced do some extra work for special system tables.
 // e.g. after inserting to the table mysql.user, we must execute `FLUSH PRIVILEGES` to allow it take effect.
-func (rc *Client) afterSystemTablesReplaced(tables []string) error {
+func (rc *Client) afterSystemTablesReplaced(ctx context.Context, tables []string) error {
 	var err error
 	for _, table := range tables {
 		if table == "user" {
 			if rc.fullClusterRestore {
 				log.Info("privilege system table restored, please reconnect to make it effective")
-				err = rc.dom.NotifyUpdatePrivilege()
+				err = multierr.Append(err, rc.dom.NotifyUpdatePrivilege())
 			} else {
 				// to make it compatible with older version
 				// todo: should we allow restore system table in non-fresh cluster in later br version?
 				// if we don't, we can check it at first place.
 				err = multierr.Append(err, errors.Annotatef(berrors.ErrUnsupportedSystemTable,
 					"restored user info may not take effect, until you should execute `FLUSH PRIVILEGES` manually"))
 			}
+		} else if table == "bind_info" {
+			if serr := rc.db.se.Execute(ctx, bindinfo.StmtRemoveDuplicatedPseudoBinding); serr != nil {
+				log.Warn("failed to delete duplicated pseudo binding", zap.Error(serr))
+				err = multierr.Append(err,
+					berrors.ErrUnknown.Wrap(serr).GenWithStack("failed to delete duplicated pseudo binding %s", bindinfo.StmtRemoveDuplicatedPseudoBinding))
+			} else {
+				log.Info("success to remove duplicated pseudo binding")
+			}
 		}
 	}
 	return err

diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go
@@ -1038,7 +1038,10 @@ func runRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConf
 
 	// The cost of rename user table / replace into system table wouldn't be so high.
 	// So leave it out of the pipeline for easier implementation.
-	client.RestoreSystemSchemas(ctx, cfg.TableFilter)
+	err = client.RestoreSystemSchemas(ctx, cfg.TableFilter)
+	if err != nil {
+		return errors.Trace(err)
+	}
 
 	schedulersRemovable = true