Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

br: skip some warning during restore system schemas #49385

Merged
merged 7 commits into from
Dec 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions br/cmd/br/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ var (
"*.*",
fmt.Sprintf("!%s.*", utils.TemporaryDBName("*")),
"!mysql.*",
"mysql.bind_info",
"mysql.user",
"mysql.db",
"mysql.tables_priv",
Expand Down
4 changes: 4 additions & 0 deletions br/cmd/br/restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/pingcap/tidb/br/pkg/trace"
"github.com/pingcap/tidb/br/pkg/utils"
"github.com/pingcap/tidb/br/pkg/version/build"
"github.com/pingcap/tidb/pkg/config"
"github.com/pingcap/tidb/pkg/session"
"github.com/pingcap/tidb/pkg/util/metricsutil"
"github.com/spf13/cobra"
Expand All @@ -38,6 +39,9 @@ func runRestoreCommand(command *cobra.Command, cmdName string) error {
}
}

// have to skip grant table, in order to NotifyUpdatePrivilege in binary mode
config.GetGlobalConfig().Security.SkipGrantTable = true

ctx := GetDefaultContext()
if cfg.EnableOpenTracing {
var store *appdash.MemoryStore
Expand Down
1 change: 0 additions & 1 deletion br/pkg/restore/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ go_library(
"//br/pkg/utils/storewatch",
"//br/pkg/version",
"//pkg/bindinfo",
"//pkg/config",
"//pkg/ddl",
"//pkg/ddl/util",
"//pkg/domain",
Expand Down
6 changes: 0 additions & 6 deletions br/pkg/restore/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ import (
"github.com/pingcap/tidb/br/pkg/utils"
"github.com/pingcap/tidb/br/pkg/utils/iter"
"github.com/pingcap/tidb/br/pkg/version"
"github.com/pingcap/tidb/pkg/config"
ddlutil "github.com/pingcap/tidb/pkg/ddl/util"
"github.com/pingcap/tidb/pkg/domain"
"github.com/pingcap/tidb/pkg/domain/infosync"
Expand Down Expand Up @@ -3537,11 +3536,6 @@ func (rc *Client) InitFullClusterRestore(explicitFilter bool) {
rc.fullClusterRestore = !explicitFilter && rc.IsFull()

log.Info("full cluster restore", zap.Bool("value", rc.fullClusterRestore))

if rc.fullClusterRestore {
// have to skip grant table, in order to NotifyUpdatePrivilege
config.GetGlobalConfig().Security.SkipGrantTable = true
}
}

func (rc *Client) IsFullClusterRestore() bool {
Expand Down
14 changes: 14 additions & 0 deletions br/pkg/restore/db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,3 +415,17 @@ func TestGetExistedUserDBs(t *testing.T) {
dbs = restore.GetExistedUserDBs(dom)
require.Equal(t, 2, len(dbs))
}

// NOTICE: Once there is a new system table, BR needs to ensure that it is correctly classified:
//
// - IF it is an unrecoverable table, please add the table name into `unRecoverableTable`.
// - IF it is an system privilege table, please add the table name into `sysPrivilegeTableMap`.
// - IF it is an statistics table, please add the table name into `statsTables`.
//
// The above variables are in the file br/pkg/restore/systable_restore.go
func TestMonitorTheSystemTableIncremental(t *testing.T) {
s := createRestoreSchemaSuite(t)
tk := testkit.NewTestKit(t, s.mock.Storage)
ret := tk.MustQuery("SELECT COUNT(*) FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_SCHEMA = 'mysql'")
ret.Equal([][]interface{}{{55}})
}
46 changes: 26 additions & 20 deletions br/pkg/restore/systable_restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,16 @@ const (
)

var statsTables = map[string]struct{}{
"stats_buckets": {},
"stats_extended": {},
"stats_feedback": {},
"stats_fm_sketch": {},
"stats_histograms": {},
"stats_meta": {},
"stats_top_n": {},
"stats_buckets": {},
"stats_extended": {},
"stats_feedback": {},
"stats_fm_sketch": {},
"stats_histograms": {},
"stats_history": {},
"stats_meta": {},
"stats_meta_history": {},
"stats_table_locked": {},
"stats_top_n": {},
}

var unRecoverableTable = map[string]struct{}{
Expand All @@ -49,6 +52,9 @@ var unRecoverableTable = map[string]struct{}{

// schema_index_usage has table id need to be rewrite.
"schema_index_usage": {},

// replace into view is not supported now
"tidb_mdl_view": {},
}

// tables in this map is restored when fullClusterRestore=true
Expand Down Expand Up @@ -149,11 +155,16 @@ 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) error {
func (rc *Client) RestoreSystemSchemas(ctx context.Context, f filter.Filter) (rerr error) {
sysDB := mysql.SystemDB

temporaryDB := utils.TemporaryDBName(sysDB)
defer rc.cleanTemporaryDatabase(ctx, sysDB)
defer func() {
// Don't clean the temporary database for next restore with checkpoint.
if rerr == nil {
rc.cleanTemporaryDatabase(ctx, sysDB)
}
}()

if !f.MatchSchema(sysDB) || !rc.withSysTable {
log.Debug("system database filtered out", zap.String("database", sysDB))
Expand Down Expand Up @@ -223,15 +234,11 @@ func (rc *Client) afterSystemTablesReplaced(ctx context.Context, tables []string
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 = multierr.Append(err, rc.dom.NotifyUpdatePrivilege())
if serr := rc.dom.NotifyUpdatePrivilege(); serr != nil {
log.Warn("failed to flush privileges, please manually execute `FLUSH PRIVILEGES`")
err = multierr.Append(err, berrors.ErrUnknown.Wrap(serr).GenWithStack("failed to flush privileges"))
} 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"))
log.Info("privilege system table restored, please reconnect to make it effective")
}
} else if table == "bind_info" {
if serr := rc.db.se.Execute(ctx, bindinfo.StmtRemoveDuplicatedPseudoBinding); serr != nil {
Expand Down Expand Up @@ -277,12 +284,11 @@ func (rc *Client) replaceTemporaryTableToSystable(ctx context.Context, ti *model
// 1.5 ) (Optional) The UPDATE statement sometimes costs, the whole system tables restore step can be place into the restore pipeline.
// 2 ) Deprecate the origin interface for backing up statistics.
if isStatsTable(tableName) {
return berrors.ErrUnsupportedSystemTable.GenWithStack("restoring stats via `mysql` schema isn't support yet: " +
"the table ID is out-of-date and may corrupt existing statistics")
return nil
}

if isUnrecoverableTable(tableName) {
return berrors.ErrUnsupportedSystemTable.GenWithStack("restoring unsupported `mysql` schema table")
return nil
}

// Currently, we don't support restore resource group metadata, so we need to
Expand Down
9 changes: 9 additions & 0 deletions br/tests/br_full_cluster_restore/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -162,5 +162,14 @@ check_contains "count(*): 0"
run_sql "select count(*) from mysql.role_edges where to_user='cloud_admin'"
check_contains "count(*): 0"

echo "--> full cluster restore with --filter, need to flush privileges"
restart_services
run_br restore full --filter "*.*" --filter "!__TiDB_BR_Temporary*.*" --filter "!mysql.*" --filter "mysql.bind_info" --filter "mysql.user" --filter "mysql.db" --filter "mysql.tables_priv" --filter "mysql.columns_priv" --filter "mysql.global_priv" --filter "mysql.global_grants" --filter "mysql.default_roles" --filter "mysql.role_edges" --filter "!sys.*" --filter "!INFORMATION_SCHEMA.*" --filter "!PERFORMANCE_SCHEMA.*" --filter "!METRICS_SCHEMA.*" --filter "!INSPECTION_SCHEMA.*" --with-sys-table --log-file $br_log_file -s "local://$backup_dir"
# BR executes `FLUSH PRIVILEGES` already
run_sql_as user1 "123456" "select count(*) from db1.t1"
check_contains "count(*): 2"
run_sql_as user1 "123456" "select count(*) from db2.t1"
check_contains "count(*): 2"

echo "clean up kept backup"
rm -rf $backup_dir $incr_backup_dir
4 changes: 2 additions & 2 deletions br/tests/br_full_ddl/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,8 @@ then
else
echo "TEST: [$TEST_NAME] fail due to stats are not equal"
grep ERROR $LOG
cat $BACKUP_STAT | head -n 1000
cat $RESOTRE_STAT | head -n 1000
cat $BACKUP_STAT | tail -n 1000
cat $RESOTRE_STAT | tail -n 1000
exit 1
fi

Expand Down