-
Notifications
You must be signed in to change notification settings - Fork 101
restore: auto remove/add pd scheduler in restore #123
Conversation
96cdc05
to
fb1ed17
Compare
/run-all-tests |
4 similar comments
/run-all-tests |
/run-all-tests |
/run-all-tests |
/run-all-tests |
Codecov Report
@@ Coverage Diff @@
## master #123 +/- ##
=======================================
Coverage 73.81% 73.81%
=======================================
Files 33 33
Lines 3483 3483
=======================================
Hits 2571 2571
Misses 589 589
Partials 323 323 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rest LGTM.
Are the schedulers still removed even in online import mode?
} | ||
ctx, cancel := context.WithCancel(GetDefaultContext()) | ||
defer cancel() | ||
func runRestore(flagSet *flag.FlagSet, cmdName, dbName, tableName string) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 👍
Sure, I think we should check is online in prepareWork |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
} | ||
|
||
func addPDLeaderScheduler(ctx context.Context, mgr *conn.Mgr, removedSchedulers []string) error { | ||
for _, scheduler := range removedSchedulers { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question. Since map is randomly ordered, the schedulers are added in random order as well. Is this OK for PD?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK, yes, there is no dependency problem between each scheduler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No description provided.