Skip to content

br: fix go leak when nothing to restore #58626

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

br: fix go leak when nothing to restore #58626

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
821bfed
fix go leak
Tristan1900 Dec 30, 2024
f7b1aba
test driven fix
Tristan1900 Jan 7, 2025
3dd5d76
remove early return
Tristan1900 Jan 9, 2025
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
15 changes: 7 additions & 8 deletions br/pkg/task/restore.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -761,6 +761,11 @@ func RunRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConf
return nil
}

func markRestoreSuccess(schedulersRemovable *bool) {
*schedulersRemovable = true
summary.SetSuccessStatus(true)
}

func runSnapshotRestore(c context.Context, mgr *conn.Mgr, g glue.Glue, cmdName string, cfg *RestoreConfig, checkInfo *PiTRTaskInfo) error {
cfg.Adjust()
defer summary.Summary(cmdName)
Expand Down Expand Up @@ -1037,9 +1042,7 @@ func runSnapshotRestore(c context.Context, mgr *conn.Mgr, g glue.Glue, cmdName s
// nothing to restore, maybe only ddl changes in incremental restore
if len(dbs) == 0 && len(tables) == 0 {
log.Info("nothing to restore, all databases and tables are filtered out")
// even nothing to restore, we show a success message since there is no failure.
summary.SetSuccessStatus(true)
return nil
// go through rest of the process, need to clean up some resources
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's strange to add comments at the rear of a code block.

}

if err = client.CreateDatabases(ctx, dbs); err != nil {
Expand Down Expand Up @@ -1067,7 +1070,6 @@ func runSnapshotRestore(c context.Context, mgr *conn.Mgr, g glue.Glue, cmdName s

if len(files) == 0 {
log.Info("no files, empty databases and tables are restored")
summary.SetSuccessStatus(true)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should go though to the end of method and get set anyway

// don't return immediately, wait all pipeline done.
} else {
oldKeyspace, _, err := tikv.DecodeKey(files[0].GetStartKey(), backupMeta.ApiVersion)
Expand Down Expand Up @@ -1192,10 +1194,7 @@ func runSnapshotRestore(c context.Context, mgr *conn.Mgr, g glue.Glue, cmdName s
return err
}

schedulersRemovable = true

// Set task summary to success status.
summary.SetSuccessStatus(true)
markRestoreSuccess(&schedulersRemovable)
return nil
}

Expand Down
10 changes: 10 additions & 0 deletions br/tests/br_table_filter/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,13 @@ run_sql "select c from $DB.FIVE;"
! run_sql 'select c from '"$DB"'.`the,special,table`;'

run_sql "drop schema $DB;"

echo "test filter out everything"
run_br restore full -f "${DB}_nothing.*" -s "local://$TEST_DIR/$DB/full" --pd $PD_ADDR
! run_sql "select c from $DB.one;"
! run_sql "select c from $DB.two;"
! run_sql "select c from $DB.three;"
! run_sql "select c from $DB.four;"
! run_sql "select c from $DB.FIVE;"
! run_sql "select c from $DB.TEN;"
! run_sql 'select c from '"$DB"'.`the,special,table`;'
Loading