Skip to content

Commit 0d74a99

Browse files
authoredMar 21, 2025
br: fix pitr system table inconsistency issue (pingcap#60202)
close pingcap#60201
1 parent c0aea84 commit 0d74a99

File tree

5 files changed

+66
-8
lines changed

5 files changed

+66
-8
lines changed
 

‎br/pkg/stream/rewrite_meta_rawkv.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,9 @@ func (sr *SchemasReplace) rewriteKeyForTable(
227227
if !exist {
228228
return nil, errors.Annotatef(berrors.ErrInvalidArgument, "failed to find table id:%v in maps", tableID)
229229
}
230-
if tableReplace.FilteredOut {
230+
231+
// don't restore meta kv change for system db, not supported yet
232+
if tableReplace.FilteredOut || utils.IsSysOrTempSysDB(dbReplace.Name) {
231233
return nil, nil
232234
}
233235

‎br/pkg/stream/table_mapping.go

+20
Original file line numberDiff line numberDiff line change
@@ -293,11 +293,31 @@ func (tm *TableMappingManager) MergeBaseDBReplace(baseMap map[UpstreamID]*DBRepl
293293
existingDBReplace.DbID = newID
294294
}
295295

296+
// db replace in `TableMappingManager` has no name yet, it is determined by baseMap.
297+
// TODO: update the name of the db replace that is not exists in baseMap.
298+
// Now it is OK because user tables' name is not used.
299+
if existingDBReplace.Name == "" {
300+
if baseDBReplace, exists := baseMap[upDBID]; exists && baseDBReplace.Name != "" {
301+
existingDBReplace.Name = baseDBReplace.Name
302+
}
303+
}
304+
296305
for upTableID, existingTableReplace := range existingDBReplace.TableMap {
297306
if newID, exists := tm.globalIdMap[upTableID]; exists {
298307
existingTableReplace.TableID = newID
299308
}
300309

310+
// table replace in `TableMappingManager` has no name yet, it is determined by baseMap.
311+
// TODO: update the name of the table replace that is not exists in baseMap.
312+
// Now it is OK because user tables' name is not used.
313+
if existingTableReplace.Name == "" {
314+
if baseDBReplace, dbExists := baseMap[upDBID]; dbExists {
315+
if baseTableReplace, tableExists := baseDBReplace.TableMap[upTableID]; tableExists && baseTableReplace.Name != "" {
316+
existingTableReplace.Name = baseTableReplace.Name
317+
}
318+
}
319+
}
320+
301321
for partUpID := range existingTableReplace.PartitionMap {
302322
if newID, exists := tm.globalIdMap[partUpID]; exists {
303323
existingTableReplace.PartitionMap[partUpID] = newID

‎br/pkg/task/stream.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -1944,7 +1944,7 @@ func buildRewriteRules(schemasReplace *stream.SchemasReplace) map[int64]*restore
19441944
rules := make(map[int64]*restoreutils.RewriteRules)
19451945

19461946
for _, dbReplace := range schemasReplace.DbReplaceMap {
1947-
if dbReplace.FilteredOut {
1947+
if dbReplace.FilteredOut || utils.IsSysOrTempSysDB(dbReplace.Name) {
19481948
continue
19491949
}
19501950
for oldTableID, tableReplace := range dbReplace.TableMap {

‎br/pkg/utils/schema.go

+8
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,14 @@ func StripTempDBPrefixIfNeeded(tempDB string) (string, bool) {
5555
return tempDB[len(temporaryDBNamePrefix):], true
5656
}
5757

58+
// IsSysOrTempSysDB tests whether the database is system DB or prefixed with temp.
59+
func IsSysOrTempSysDB(db string) bool {
60+
if name, ok := StripTempDBPrefixIfNeeded(db); IsSysDB(name) && ok {
61+
return true
62+
}
63+
return false
64+
}
65+
5866
// GetSysDBCIStrName get the CIStr name of system DB
5967
func GetSysDBCIStrName(tempDB ast.CIStr) (ast.CIStr, bool) {
6068
if ok := strings.HasPrefix(tempDB.O, temporaryDBNamePrefix); !ok {

‎br/tests/br_pitr_table_filter/run.sh

+34-6
Original file line numberDiff line numberDiff line change
@@ -450,25 +450,53 @@ test_system_tables() {
450450
run_sql "create schema $DB;"
451451

452452
echo "write initial data and do snapshot backup"
453-
# create and populate a user table for reference
454-
run_sql "create table $DB.user_table(id int primary key);"
455-
run_sql "insert into $DB.user_table values (1);"
456-
457453
# make some changes to system tables
458454
run_sql "create user 'test_user'@'%' identified by 'password';"
459455
run_sql "grant select on $DB.* to 'test_user'@'%';"
460456

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

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

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

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

467+
echo "Test 1: Verify that default restore behavior (no filter) properly handles system tables"
468+
# restore without any filter, should only restore snapshot system tables, not log backup.
469+
# this is the current behavior as restore log backup to system table will have issue
470+
run_br --pd "$PD_ADDR" restore point -s "local://$TEST_DIR/$TASK_NAME/log" --full-backup-storage "local://$TEST_DIR/$TASK_NAME/full"
471+
472+
473+
# verify system tables are restored from snapshot only
474+
# only test_user should exist, post_backup_user should not exist
475+
users_result=$(run_sql "SELECT _tidb_rowid, user, host, authentication_string FROM mysql.user WHERE user IN ('test_user', 'post_backup_user')")
476+
477+
test_user_count=$(echo "$users_result" | grep -c "test_user" || true)
478+
479+
# Verify there is exactly one test_user
480+
if [ "$test_user_count" -eq 0 ]; then
481+
echo "Error: test_user not found in mysql.user table"
482+
exit 1
483+
elif [ "$test_user_count" -gt 1 ]; then
484+
echo "Error: Found $test_user_count instances of test_user in mysql.user table, expected exactly 1"
485+
echo "Full query result:"
486+
echo "$users_result"
487+
exit 1
488+
fi
489+
490+
# Check that post_backup_user does not exist (was created after snapshot)
491+
if echo "$users_result" | grep -q "post_backup_user"; then
492+
echo "Error: post_backup_user found in mysql.user table but should not be restored"
493+
echo "Full query result:"
494+
echo "$users_result"
495+
exit 1
496+
fi
497+
498+
echo "Default restore correctly restored system tables from snapshot only: verified one test_user exists"
499+
472500
echo "PiTR should error out when system tables are included with explicit filter"
473501
restore_fail=0
474502
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

0 commit comments

Comments
 (0)
Please sign in to comment.