Skip to content

br: fix pitr system table inconsistency issue #60202

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

Merged
merged 8 commits into from
Mar 21, 2025
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion br/pkg/stream/rewrite_meta_rawkv.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,9 @@ func (sr *SchemasReplace) rewriteKeyForTable(
if !exist {
return nil, errors.Annotatef(berrors.ErrInvalidArgument, "failed to find table id:%v in maps", tableID)
}
if tableReplace.FilteredOut {

// don't restore meta kv change for system db, not supported yet
if tableReplace.FilteredOut || utils.IsSysOrTempSysDB(dbReplace.Name) {
return nil, nil
}

Expand Down
18 changes: 18 additions & 0 deletions br/pkg/stream/table_mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,11 +293,29 @@ func (tm *TableMappingManager) MergeBaseDBReplace(baseMap map[UpstreamID]*DBRepl
existingDBReplace.DbID = newID
}

// db replace in `TableMappingManager` has no name yet, it is determined by baseMap.
// TODO: update the name of the db replace that is not exists in baseMap. Now it is OK because user tables' name is not used.
if existingDBReplace.Name == "" {
if baseDBReplace, exists := baseMap[upDBID]; exists && baseDBReplace.Name != "" {
existingDBReplace.Name = baseDBReplace.Name
}
}

for upTableID, existingTableReplace := range existingDBReplace.TableMap {
if newID, exists := tm.globalIdMap[upTableID]; exists {
existingTableReplace.TableID = newID
}

// table replace in `TableMappingManager` has no name yet, it is determined by baseMap.
// TODO: update the name of the table replace that is not exists in baseMap. Now it is OK because user tables' name is not used.
if existingTableReplace.Name == "" {
if baseDBReplace, dbExists := baseMap[upDBID]; dbExists {
if baseTableReplace, tableExists := baseDBReplace.TableMap[upTableID]; tableExists && baseTableReplace.Name != "" {
existingTableReplace.Name = baseTableReplace.Name
}
}
}

for partUpID := range existingTableReplace.PartitionMap {
if newID, exists := tm.globalIdMap[partUpID]; exists {
existingTableReplace.PartitionMap[partUpID] = newID
Expand Down
2 changes: 1 addition & 1 deletion br/pkg/task/stream.go
Original file line number Diff line number Diff line change
Expand Up @@ -1944,7 +1944,7 @@ func buildRewriteRules(schemasReplace *stream.SchemasReplace) map[int64]*restore
rules := make(map[int64]*restoreutils.RewriteRules)

for _, dbReplace := range schemasReplace.DbReplaceMap {
if dbReplace.FilteredOut {
if dbReplace.FilteredOut || utils.IsSysOrTempSysDB(dbReplace.Name) {
continue
}
for oldTableID, tableReplace := range dbReplace.TableMap {
Expand Down
8 changes: 8 additions & 0 deletions br/pkg/utils/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ func StripTempDBPrefixIfNeeded(tempDB string) (string, bool) {
return tempDB[len(temporaryDBNamePrefix):], true
}

// IsSysOrTempSysDB tests whether the database is system DB or prefixed with temp.
func IsSysOrTempSysDB(db string) bool {
if name, ok := StripTempDBPrefixIfNeeded(db); IsSysDB(name) && ok {
return true
}
return false
}

// GetSysDBCIStrName get the CIStr name of system DB
func GetSysDBCIStrName(tempDB ast.CIStr) (ast.CIStr, bool) {
if ok := strings.HasPrefix(tempDB.O, temporaryDBNamePrefix); !ok {
Expand Down
40 changes: 34 additions & 6 deletions br/tests/br_pitr_table_filter/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -450,25 +450,53 @@ test_system_tables() {
run_sql "create schema $DB;"

echo "write initial data and do snapshot backup"
# create and populate a user table for reference
run_sql "create table $DB.user_table(id int primary key);"
run_sql "insert into $DB.user_table values (1);"

# make some changes to system tables
run_sql "create user 'test_user'@'%' identified by 'password';"
run_sql "grant select on $DB.* to 'test_user'@'%';"

run_br backup full -s "local://$TEST_DIR/$TASK_NAME/full" --pd $PD_ADDR

echo "make more changes to system tables and wait for log backup"
run_sql "revoke select on $DB.* from 'test_user'@'%';"
run_sql "grant insert on $DB.* to 'test_user'@'%';"
run_sql "create user 'post_backup_user'@'%' identified by 'otherpassword';"
run_sql "alter user 'test_user'@'%' identified by 'newpassword';"

. "$CUR/../br_test_utils.sh" && wait_log_checkpoint_advance "$TASK_NAME"

restart_services || { echo "Failed to restart services"; exit 1; }

echo "Test 1: Verify that default restore behavior (no filter) properly handles system tables"
# restore without any filter, should only restore snapshot system tables, not log backup.
# this is the current behavior as restore log backup to system table will have issue
run_br --pd "$PD_ADDR" restore point -s "local://$TEST_DIR/$TASK_NAME/log" --full-backup-storage "local://$TEST_DIR/$TASK_NAME/full"


# verify system tables are restored from snapshot only
# only test_user should exist, post_backup_user should not exist
users_result=$(run_sql "SELECT _tidb_rowid, user, host, authentication_string FROM mysql.user WHERE user IN ('test_user', 'post_backup_user')")

test_user_count=$(echo "$users_result" | grep -c "test_user" || true)

# Verify there is exactly one test_user
if [ "$test_user_count" -eq 0 ]; then
echo "Error: test_user not found in mysql.user table"
exit 1
elif [ "$test_user_count" -gt 1 ]; then
echo "Error: Found $test_user_count instances of test_user in mysql.user table, expected exactly 1"
echo "Full query result:"
echo "$users_result"
exit 1
fi

# Check that post_backup_user does not exist (was created after snapshot)
if echo "$users_result" | grep -q "post_backup_user"; then
echo "Error: post_backup_user found in mysql.user table but should not be restored"
echo "Full query result:"
echo "$users_result"
exit 1
fi

echo "Default restore correctly restored system tables from snapshot only: verified one test_user exists"

echo "PiTR should error out when system tables are included with explicit filter"
restore_fail=0
run_br --pd "$PD_ADDR" restore point -f "*.*" -s "local://$TEST_DIR/$TASK_NAME/log" --full-backup-storage "local://$TEST_DIR/$TASK_NAME/full" || restore_fail=1
Expand Down
Loading