-
Notifications
You must be signed in to change notification settings - Fork 721
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
scheduler: add disable to independent config #8567
Changes from 1 commit
5667cbc
83c7fcf
32fc044
0efe633
19d1615
2765b8c
590f2c1
8963a49
25120c4
14454b5
456bd02
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,7 +33,6 @@ | |
"github.com/tikv/pd/pkg/schedule/plan" | ||
"github.com/tikv/pd/pkg/schedule/types" | ||
"github.com/tikv/pd/pkg/utils/reflectutil" | ||
"github.com/tikv/pd/pkg/utils/syncutil" | ||
"github.com/tikv/pd/pkg/utils/typeutil" | ||
"github.com/unrolled/render" | ||
"go.uber.org/zap" | ||
|
@@ -52,8 +51,7 @@ | |
) | ||
|
||
type balanceLeaderSchedulerConfig struct { | ||
syncutil.RWMutex | ||
schedulerConfig | ||
baseDefaultSchedulerConfig | ||
|
||
Ranges []core.KeyRange `json:"ranges"` | ||
// Batch is used to generate multiple operators by one scheduling | ||
|
@@ -164,7 +162,7 @@ | |
// each store balanced. | ||
func newBalanceLeaderScheduler(opController *operator.Controller, conf *balanceLeaderSchedulerConfig, options ...BalanceLeaderCreateOption) Scheduler { | ||
s := &balanceLeaderScheduler{ | ||
BaseScheduler: NewBaseScheduler(opController, types.BalanceLeaderScheduler), | ||
BaseScheduler: NewBaseScheduler(opController, types.BalanceLeaderScheduler, conf), | ||
retryQuota: newRetryQuota(), | ||
conf: conf, | ||
handler: newBalanceLeaderHandler(conf), | ||
|
@@ -541,3 +539,18 @@ | |
op.SetAdditionalInfo("targetScore", strconv.FormatFloat(solver.targetScore, 'f', 2, 64)) | ||
return op | ||
} | ||
|
||
// IsDiable implements the Scheduler interface. | ||
func (l *balanceLeaderScheduler) IsDisable() bool { | ||
return l.conf.isDisable() | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to implement it for every scheduler? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. base_scheduler.go implemented it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What I mean is can we implement this common function only once. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we want to implement it once, we need to put There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WDYT? Is it possible that we might customize it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this pr, we only need to make a customized implementation for defaultScheduler. I think this is acceptable and only needs to be implemented four times. If follow what you said, it does only need to be implemented once, but I feel that |
||
|
||
// SetDiable implements the Scheduler interface. | ||
func (l *balanceLeaderScheduler) SetDisable(disable bool) error { | ||
return l.conf.setDisable(disable) | ||
} | ||
|
||
// IsDefault implements the Scheduler interface. | ||
func (*balanceLeaderScheduler) IsDefault() bool { | ||
return true | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,11 +65,16 @@ | |
|
||
name string | ||
tp types.CheckerSchedulerType | ||
conf schedulerConfig | ||
} | ||
|
||
// NewBaseScheduler returns a basic scheduler | ||
func NewBaseScheduler(opController *operator.Controller, tp types.CheckerSchedulerType) *BaseScheduler { | ||
return &BaseScheduler{OpController: opController, tp: tp} | ||
func NewBaseScheduler( | ||
opController *operator.Controller, | ||
tp types.CheckerSchedulerType, | ||
conf schedulerConfig, | ||
) *BaseScheduler { | ||
return &BaseScheduler{OpController: opController, tp: tp, conf: conf} | ||
} | ||
|
||
func (*BaseScheduler) ServeHTTP(w http.ResponseWriter, _ *http.Request) { | ||
|
@@ -114,3 +119,12 @@ | |
func (s *BaseScheduler) GetType() types.CheckerSchedulerType { | ||
return s.tp | ||
} | ||
|
||
// IsDiable implements the Scheduler interface. | ||
func (*BaseScheduler) IsDisable() bool { return false } | ||
|
||
// SetDisable implements the Scheduler interface. | ||
func (*BaseScheduler) SetDisable(bool) error { return nil } | ||
|
||
// IsDefault returns if the scheduler is a default scheduler. | ||
func (*BaseScheduler) IsDefault() bool { return false } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about:
then no need to rewrite the interface for default 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.
I suggest moving
Disabled
here.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.
If this, we need implement
isDisable
andsetDisable
four times.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.
friendly ping~
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.
Just like other config?