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

Standardize the use of scheduler name/type and checker name/type #8379

Closed
okJiang opened this issue Jul 9, 2024 · 0 comments · Fixed by #8607
Closed

Standardize the use of scheduler name/type and checker name/type #8379

okJiang opened this issue Jul 9, 2024 · 0 comments · Fixed by #8607
Labels
type/enhancement The issue or PR belongs to an enhancement.

Comments

@okJiang
Copy link
Member

okJiang commented Jul 9, 2024

Target

  • Use a unified naming convention for schedulers/checkers
  • Standardize the concepts of name and type of scheduler and converge their usage

Naming summary

  • xxxx_checker this underscore format only appears in the checker, scheduler does not. It is recommended to unify it into the format of xxxx-checker
  • Xx-y-scheduler (schedulerName) and xx-yy (schedulerType) are unified into xx-y-scheduler. Reason
    • The interaction with users uses the former. If the latter is used, it will destroy the consistency of user usage or generate a lot of additional work (do a lot of converts) to maintain user consistency, increasing complexity
    • The former has more usage scenarios in the code than the latter
    • The former will be persistent, the latter will not
    • Unified with xxxx-checker
  • Operator desc
    • Checker uses xxxx-checker.
    • Scheduler uses xxxx-yyyy. It will be replaced with xxxx-yyy-scheduler. It affects the printing of logs

checker

  • xxxx_checker(checkerName)
  • xxxx-checker(checkerType)

scheduler

  • schedulerName is in the format of xx-y-scheduler
  • schedulerType is in the format of xx-yy
  • In addition, there are two special ones
    // HotRegionName is balance hot region scheduler name.
    HotRegionName = "balance-hot-region-scheduler"
    // HotRegionType is balance hot region scheduler type.
    HotRegionType          = "hot-region"
        // ScatterRangeType is scatter range scheduler type
    ScatterRangeType = "scatter-range"
    // ScatterRangeName is scatter range scheduler name
    ScatterRangeName = "scatter-range"
  • xx-yy-scheduler(schedulerName)
  • xx-yy(schedulerType)

Deep scheduler name/type

For schedulers, there is indeed a difference between name and type, but in the current code, these two are completely confused, making it difficult for readers to understand them. This section aims to completely distinguish them and give a clear definition. First

name

There are three ways to name a name

WithXxxxxxName

These three schedulers allow you to specify their names at creation time.

  • balanceLeaderScheduler, not used yet
  • balanceRegionScheduler, will specify the name as scatter-range-region
  • balanceWitnessScheduler, will specify the name as scatter-range-leader

ScatterRange

Scatter-range-scheduler name is named by the key-range entered by the user, named scatter-range- {key-range} . Later changed to scatter-range-scheduler- {key-range}

Other

The names of other schedulers can be directly equivalent to type because they are constant

Name vs type

  • Type can be directly understood as the types of schedulers we support.
var string2SchedulerType = 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,
    "scatter-range-scheduler":           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,
}
  • Name is the name of a scheduler at runtime, and it must be unique in the code execution. Because it is impossible to run two identical schedulers at the same time. So with this definition, it should be used in these places:
    • Stored in the scheduler config (storage). Each schedudler at runtime should have its own configuration.
    • The Controller will contain all runtime schedulers/scheduler-handles, and each scheduler will have a corresponding name.
    • The scope in the filter needs to use name, which also records the runtime state

Compatibility issues

  • Keep the original monitoring and add the modified monitoring.
  • schedulerConfig will save (xxxx-yyyy) schedulerType in PersistOptions, and then persist it to storage. It is necessary to find the form of schedulerType when reloading and then re-Set it to the form of xxxx-yyyy-scheduler to ensure compatibility.
  • Scatter-range will change to scatter-range-scheduler , which needs to ensure compatibility and affect pd-ctl and http client. You need to keep the previous scatter-range command and use scatter-range-scheduler in the documentation (fortunately, there is no explanation of the scatter-range command in the pd-ctl documentation now - -)

Conclusions

After some offline discussions, here are some conclusions.

  • xxxx_checker and xxxx-checker unified into xxxx-checker
  • Xx-y-scheduler and xx-yy are unified into xx-y-scheduler.
  • Standardize the use of scheduler name, use scheduler.Name to access scheduler name instead of global constant
  • For scatter-range, keep the scatter-range command in API/pdctl and add scatter-range-scheduler command
  • For schedulerConfig that needs to be persisted, convert to the old format to maintain compatibility
  • For monitoring, it will be used as a filter rule_checker/replica_checker/merge_checker , temporarily converted from xxxx-checker to xxxx_checker when needed to maintain user experience compatibility. If it is determined that there is no compatibility problem, then change it
  • Other monitoring will not be used as filters, but will only be grouped and will only affect the displayed name (some change from xx-yy to xx-yy-scheduler), but will not affect the displayed content. There are no compatibility issues

Code changes

ref #8316

There are too many code changes. After splitting PR, review them one by one and merge them into the feature branch, and then merge them into the master.

  • [Done] All xxxxxName / xxxxxType are unified as XxxxChecker / XxxxScheduler , that is, xxxxx-checker / xxxxx-scheduler form
  • [Done] Modify XxxxScheduler. GetName () to XxxxScheduler.Name () and use the latter to represent the semantics of SchedulerName
  • [Done] Remove XxxxScheduler. GetType ()
  • [Done] CheckSchedulerType to check if schedulerName is a supported schedulerType
  • [Done] Convert schedulerName to schedulerType through ConvertSchedulerStr2Type
  • [Done] Remove String () from filter, use XxxxChecker / XxxxScheduler uniformly
  • [Done] Scatter-range will be converted to scatter-range-scheduler , which will affect pd-ctl and http-client
  • TODO: scatter-range-scheduler Compatibility Makeover

Progress

@okJiang okJiang added the type/enhancement The issue or PR belongs to an enhancement. label Jul 9, 2024
@okJiang okJiang changed the title Unify scheduler name/type and checker name/type Standardize the use of scheduler name/type and checker name/type Jul 9, 2024
ti-chi-bot bot pushed a commit that referenced this issue Jul 15, 2024
ref #8379

checker: use CheckerSchedulerType for checker
- move metrics to proper place
- rename GetType() to Name()

Signed-off-by: okJiang <819421878@qq.com>
ti-chi-bot bot added a commit that referenced this issue Jul 17, 2024
ref #8379

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
ti-chi-bot bot added a commit that referenced this issue Jul 25, 2024
…ckerSchedulerType. (#8416)

ref #8379

1. Replace the input parameter of AddScheduler with types.CheckerSchedulerType.
2. Simplified the logic of AddScheduler by using a temporary map.
3. Remove scope's redundant definition and use types.CheckerSchedulerType.

Signed-off-by: okJiang <819421878@qq.com>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
ti-chi-bot bot added a commit that referenced this issue Aug 1, 2024
ref #8379

- GetType() returns types.CheckerSchedulerType directly
- keep the compatibility in `PersistOptions` and `PersistConfig“
- wrap `operator.OperatorLimitCounter.WithLabelValues().Inc()` to func `IncOperatorLimitCounter`

Signed-off-by: okJiang <819421878@qq.com>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
ti-chi-bot bot pushed a commit that referenced this issue Aug 8, 2024
ref #8379

- Remove the old scheduler type, such as BalanceLeaderType
- Instead old scheduler type string of types.CheckerSchedulerType
- Instead old scheduler name of types.CheckerSchedulerType in SchedulerRegisterMap

Signed-off-by: okJiang <819421878@qq.com>
ti-chi-bot bot pushed a commit that referenced this issue Aug 14, 2024
ti-chi-bot bot added a commit that referenced this issue Aug 21, 2024
ref #8379

Signed-off-by: okJiang <819421878@qq.com>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
ti-chi-bot bot added a commit that referenced this issue Sep 24, 2024
close #8379

Signed-off-by: okJiang <819421878@qq.com>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
ti-chi-bot bot pushed a commit that referenced this issue Sep 29, 2024
ref #8379

Signed-off-by: Ryan Leung <rleungx@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant