Skip to content
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: Replace the input parameter of AddScheduler with types.CheckerSchedulerType. #8416

Merged
merged 7 commits into from
Jul 25, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
60 changes: 0 additions & 60 deletions pkg/schedule/filter/counter.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,66 +40,6 @@ func (a action) String() string {
return "unknown"
}

type scope int

const (
// BalanceLeader is the filter type for balance leader.
BalanceLeader scope = iota
// BalanceRegion is the filter type for balance region.
BalanceRegion
// BalanceHotRegion is the filter type for hot region.
BalanceHotRegion
// BalanceWitness is the filter type for balance witness.
BalanceWitness
// Label is the filter type for replica.
Label

// EvictLeader is the filter type for evict leader.
EvictLeader
// RegionScatter is the filter type for scatter region.
RegionScatter
// ReplicaChecker is the filter type for replica.
ReplicaChecker
// RuleChecker is the filter type for rule.
RuleChecker

// GrantHotLeader is the filter type for grant hot leader.
GrantHotLeader
// ShuffleHotRegion is the filter type for shuffle hot region.
ShuffleHotRegion
// ShuffleRegion is the filter type for shuffle region.
ShuffleRegion
// RandomMerge is the filter type for random merge.
RandomMerge
scopeLen
)

var scopes = [scopeLen]string{
"balance-leader-scheduler",
"balance-region-scheduler",
"balance-hot-region-scheduler",
"balance-witness-scheduler",
"label-scheduler",

"evict-leader-scheduler",
"region-scatter",
"replica-checker",
"rule-checker",

"grant-hot-leader-scheduler",
"shuffle-region-scheduler",
"shuffle-region-scheduler",
"random-merge-scheduler",
}

// String implements fmt.Stringer interface.
func (s scope) String() string {
if s >= scopeLen {
return "unknown"
}
return scopes[s]
}

type filterType int

const (
Expand Down
3 changes: 2 additions & 1 deletion pkg/schedule/filter/counter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"testing"

"github.com/stretchr/testify/require"
types "github.com/tikv/pd/pkg/schedule/type"
)

func TestString(t *testing.T) {
Expand All @@ -39,7 +40,7 @@ func TestString(t *testing.T) {

func TestCounter(t *testing.T) {
re := require.New(t)
counter := NewCounter(BalanceLeader.String())
counter := NewCounter(types.BalanceLeaderScheduler.String())
counter.inc(source, storeStateTombstone, 1, 2)
counter.inc(target, storeStateTombstone, 1, 2)
re.Equal(1, counter.counter[source][storeStateTombstone][1][2])
Expand Down
3 changes: 2 additions & 1 deletion pkg/schedule/schedulers/balance_leader.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/tikv/pd/pkg/schedule/filter"
"github.com/tikv/pd/pkg/schedule/operator"
"github.com/tikv/pd/pkg/schedule/plan"
types "github.com/tikv/pd/pkg/schedule/type"
"github.com/tikv/pd/pkg/storage/endpoint"
"github.com/tikv/pd/pkg/utils/reflectutil"
"github.com/tikv/pd/pkg/utils/syncutil"
Expand Down Expand Up @@ -182,7 +183,7 @@ func newBalanceLeaderScheduler(opController *operator.Controller, conf *balanceL
name: BalanceLeaderName,
conf: conf,
handler: newBalanceLeaderHandler(conf),
filterCounter: filter.NewCounter(filter.BalanceLeader.String()),
filterCounter: filter.NewCounter(types.BalanceLeaderScheduler.String()),
}
for _, option := range options {
option(s)
Expand Down
3 changes: 2 additions & 1 deletion pkg/schedule/schedulers/balance_region.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/tikv/pd/pkg/schedule/filter"
"github.com/tikv/pd/pkg/schedule/operator"
"github.com/tikv/pd/pkg/schedule/plan"
types "github.com/tikv/pd/pkg/schedule/type"
"go.uber.org/zap"
)

Expand Down Expand Up @@ -58,7 +59,7 @@ func newBalanceRegionScheduler(opController *operator.Controller, conf *balanceR
BaseScheduler: base,
retryQuota: newRetryQuota(),
conf: conf,
filterCounter: filter.NewCounter(filter.BalanceRegion.String()),
filterCounter: filter.NewCounter(types.BalanceRegionScheduler.String()),
}
for _, setOption := range opts {
setOption(scheduler)
Expand Down
3 changes: 2 additions & 1 deletion pkg/schedule/schedulers/balance_witness.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/tikv/pd/pkg/schedule/filter"
"github.com/tikv/pd/pkg/schedule/operator"
"github.com/tikv/pd/pkg/schedule/plan"
types "github.com/tikv/pd/pkg/schedule/type"
"github.com/tikv/pd/pkg/storage/endpoint"
"github.com/tikv/pd/pkg/utils/reflectutil"
"github.com/tikv/pd/pkg/utils/syncutil"
Expand Down Expand Up @@ -181,7 +182,7 @@ func newBalanceWitnessScheduler(opController *operator.Controller, conf *balance
conf: conf,
handler: newBalanceWitnessHandler(conf),
counter: balanceWitnessCounter,
filterCounter: filter.NewCounter(filter.BalanceWitness.String()),
filterCounter: filter.NewCounter(types.BalanceWitnessScheduler.String()),
}
for _, option := range options {
option(s)
Expand Down
43 changes: 43 additions & 0 deletions pkg/schedule/type/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,46 @@ const (
// LabelScheduler is label scheduler name.
LabelScheduler CheckerSchedulerType = "label-scheduler"
)

// SchedulerTypeCompatibleMap temporarily exists for compatibility.
Copy link
Member

Choose a reason for hiding this comment

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

I suggest adding more explanation about when we need to consider the compatibility.

Copy link
Member Author

@okJiang okJiang Jul 19, 2024

Choose a reason for hiding this comment

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

I don't think there is a need to explain in the code now. Its current role is only to do a layer of conversion when the user adds a scheduler. If it is used in more places in the future, or permanently exists in the code, I will add a more accurate description to it. How about this?

Copy link
Member Author

Choose a reason for hiding this comment

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

ping @rleungx

Copy link
Member

@rleungx rleungx Jul 23, 2024

Choose a reason for hiding this comment

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

I am just wondering when we need to consider the compatibility.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is an intermediate pr, it may be deleted in the subsequent pr, if it stays, I will add more comments

// TODO: remove it after all components use CheckerSchedulerType.
var SchedulerTypeCompatibleMap = map[CheckerSchedulerType]string{
BalanceLeaderScheduler: "balance-leader",
BalanceRegionScheduler: "balance-region",
BalanceWitnessScheduler: "balance-witness",
EvictLeaderScheduler: "evict-leader",
EvictSlowStoreScheduler: "evict-slow-store",
EvictSlowTrendScheduler: "evict-slow-trend",
GrantLeaderScheduler: "grant-leader",
GrantHotRegionScheduler: "grant-hot-region",
HotRegionScheduler: "hot-region",
RandomMergeScheduler: "random-merge",
ScatterRangeScheduler: "scatter-range",
ShuffleHotRegionScheduler: "shuffle-hot-region",
ShuffleLeaderScheduler: "shuffle-leader",
ShuffleRegionScheduler: "shuffle-region",
SplitBucketScheduler: "split-bucket",
TransferWitnessLeaderScheduler: "transfer-witness-leader",
LabelScheduler: "label",
}

var SchedulerStr2Type = map[string]CheckerSchedulerType{
"balance-leader-scheduler": BalanceLeaderScheduler,
"balance-region-scheduler": BalanceRegionScheduler,
"balance-witness-scheduler": BalanceWitnessScheduler,
"evict-leader-scheduler": EvictLeaderScheduler,
"evict-slow-store-scheduler": EvictSlowStoreScheduler,
"evict-slow-trend-scheduler": EvictSlowTrendScheduler,
"grant-leader-scheduler": GrantLeaderScheduler,
"grant-hot-region-scheduler": GrantHotRegionScheduler,
"balance-hot-region-scheduler": HotRegionScheduler,
"random-merge-scheduler": RandomMergeScheduler,
// TODO: update to `scatter-range-scheduler`
"scatter-range": ScatterRangeScheduler,
"shuffle-hot-region-scheduler": ShuffleHotRegionScheduler,
"shuffle-leader-scheduler": ShuffleLeaderScheduler,
"shuffle-region-scheduler": ShuffleRegionScheduler,
"split-bucket-scheduler": SplitBucketScheduler,
"transfer-witness-leader-scheduler": TransferWitnessLeaderScheduler,
"label-scheduler": LabelScheduler,
}
127 changes: 38 additions & 89 deletions server/api/scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,19 @@
import (
"net/http"
"net/url"
"strconv"
"strings"

"github.com/gorilla/mux"
"github.com/pingcap/errors"
"github.com/pingcap/log"
"github.com/tikv/pd/pkg/errs"
"github.com/tikv/pd/pkg/schedule/schedulers"
types "github.com/tikv/pd/pkg/schedule/type"
"github.com/tikv/pd/pkg/utils/apiutil"
"github.com/tikv/pd/server"
"github.com/unrolled/render"
"go.uber.org/zap"
)

type schedulerHandler struct {
Expand Down Expand Up @@ -81,48 +85,18 @@
return
}

switch name {
case schedulers.BalanceLeaderName:
if err := h.AddBalanceLeaderScheduler(); err != nil {
h.r.JSON(w, http.StatusInternalServerError, err.Error())
return
}
case schedulers.BalanceWitnessName:
if err := h.AddBalanceWitnessScheduler(); err != nil {
h.r.JSON(w, http.StatusInternalServerError, err.Error())
return
}
case schedulers.TransferWitnessLeaderName:
if err := h.AddTransferWitnessLeaderScheduler(); err != nil {
h.r.JSON(w, http.StatusInternalServerError, err.Error())
return
}
case schedulers.HotRegionName:
if err := h.AddBalanceHotRegionScheduler(); err != nil {
h.r.JSON(w, http.StatusInternalServerError, err.Error())
return
}
case schedulers.EvictSlowTrendName:
if err := h.AddEvictSlowTrendScheduler(); err != nil {
h.r.JSON(w, http.StatusInternalServerError, err.Error())
return
}
case schedulers.BalanceRegionName:
if err := h.AddBalanceRegionScheduler(); err != nil {
h.r.JSON(w, http.StatusInternalServerError, err.Error())
return
}
case schedulers.LabelName:
if err := h.AddLabelScheduler(); err != nil {
h.r.JSON(w, http.StatusInternalServerError, err.Error())
return
}
case schedulers.ScatterRangeName:
var args []string
tp, ok := types.SchedulerStr2Type[name]
if !ok {
h.r.JSON(w, http.StatusBadRequest, "unknown scheduler")
return

Check warning on line 91 in server/api/scheduler.go

View check run for this annotation

Codecov / codecov/patch

server/api/scheduler.go#L90-L91

Added lines #L90 - L91 were not covered by tests
}
var args []string
collector := func(v string) {
args = append(args, v)
}

collector := func(v string) {
args = append(args, v)
}
switch tp {
case types.ScatterRangeScheduler:
if err := apiutil.CollectEscapeStringOption("start_key", input, collector); err != nil {
h.r.JSON(w, http.StatusInternalServerError, err.Error())
return
Expand All @@ -137,64 +111,39 @@
h.r.JSON(w, http.StatusInternalServerError, err.Error())
return
}
if err := h.AddScatterRangeScheduler(args...); err != nil {
h.r.JSON(w, http.StatusInternalServerError, err.Error())
return
}

case schedulers.GrantLeaderName, schedulers.EvictLeaderName:
case types.GrantLeaderScheduler, types.EvictLeaderScheduler:
storeID, ok := input["store_id"].(float64)
if !ok {
h.r.JSON(w, http.StatusBadRequest, "missing store id")
return
}
exist, err := h.AddEvictOrGrant(storeID, name)
if err != nil {
h.r.JSON(w, http.StatusInternalServerError, err.Error())
return
}
// we should ensure whether it is the first time to create evict-leader-scheduler
// or just update the evict-leader.
if exist {
var (
exist bool
err error
)
if exist, err = h.IsSchedulerExisted(name); exist {
if err := h.RedirectSchedulerUpdate(name, storeID); err != nil {
h.r.JSON(w, http.StatusInternalServerError, err.Error())
return
}
log.Info("update scheduler", zap.String("scheduler-name", name), zap.Uint64("store-id", uint64(storeID)))
h.r.JSON(w, http.StatusOK, "The scheduler has been applied to the store.")
return
}
case schedulers.ShuffleLeaderName:
if err := h.AddShuffleLeaderScheduler(); err != nil {
h.r.JSON(w, http.StatusInternalServerError, err.Error())
return
}
case schedulers.ShuffleRegionName:
if err := h.AddShuffleRegionScheduler(); err != nil {
if err != nil && !errors.ErrorEqual(err, errs.ErrSchedulerNotFound.FastGenByArgs()) {
h.r.JSON(w, http.StatusInternalServerError, err.Error())
return
}
case schedulers.RandomMergeName:
if err := h.AddRandomMergeScheduler(); err != nil {
h.r.JSON(w, http.StatusInternalServerError, err.Error())
return
}
case schedulers.ShuffleHotRegionName:

collector(strconv.FormatUint(uint64(storeID), 10))
case types.ShuffleHotRegionScheduler:
limit := uint64(1)
l, ok := input["limit"].(float64)
if ok {
limit = uint64(l)
}
if err := h.AddShuffleHotRegionScheduler(limit); err != nil {
h.r.JSON(w, http.StatusInternalServerError, err.Error())
return
}
case schedulers.EvictSlowStoreName:
if err := h.AddEvictSlowStoreScheduler(); err != nil {
h.r.JSON(w, http.StatusInternalServerError, err.Error())
return
}
case schedulers.SplitBucketName:
if err := h.AddSplitBucketScheduler(); err != nil {
h.r.JSON(w, http.StatusInternalServerError, err.Error())
return
}
case schedulers.GrantHotRegionName:
collector(strconv.FormatUint(limit, 10))
case types.GrantHotRegionScheduler:
leaderID, ok := input["store-leader-id"].(string)
if !ok {
h.r.JSON(w, http.StatusBadRequest, "missing leader id")
Expand All @@ -205,12 +154,12 @@
h.r.JSON(w, http.StatusBadRequest, "missing store id")
return
}
if err := h.AddGrantHotRegionScheduler(leaderID, peerIDs); err != nil {
h.r.JSON(w, http.StatusInternalServerError, err.Error())
return
}
default:
h.r.JSON(w, http.StatusBadRequest, "unknown scheduler")
collector(leaderID)
collector(peerIDs)
}

if err := h.AddScheduler(tp, args...); err != nil {
h.r.JSON(w, http.StatusInternalServerError, err.Error())
return
}

Expand Down
Loading