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

Dynamic loading comfigmap about action and plugins of scheduler, move loadSchedulerConf processing from run to runOnce #504

Merged
merged 1 commit into from
Nov 1, 2019

Conversation

sivanzcw
Copy link
Contributor

@sivanzcw sivanzcw commented Nov 1, 2019

The reason for this changes is to realize dynamically loading configuration of scheduler. When configuration about action or plugins of scheduler changes, the scheduler can load and use the new configuration without restarting.

For time of loading configuration is very shot, the changes has no effect on the scheduling performance

Time duration of scheduler.loadSchedulerConf function

serial num time duration(us)
1 226.97
2 179.438
3 181.991
4 326.252
5 331.443
6 256.802
7 325.434
8 254.457
9 504.386
10 216.369
11 221.776
12 266.722
13 211.502
14 203.506
15 208.086
16 233.679
17 241.664
18 234.157
19 200.129
20 198.543

@volcano-sh-bot volcano-sh-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 1, 2019
@sivanzcw sivanzcw changed the title Dynamic loading comfigmap about action and plugins of schedule, move loadSchedulerConf processing from run to runOnce Dynamic loading comfigmap about action and plugins of scheduler, move loadSchedulerConf processing from run to runOnce Nov 1, 2019
@TravisBuddy
Copy link

Hey @sivanzcw,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 32769770-fc8a-11e9-afa1-7d9fdd5cb875

@k82cn
Copy link
Member

k82cn commented Nov 1, 2019

/lgtm
/approve

@volcano-sh-bot volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 1, 2019
@volcano-sh-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: k82cn, sivanzcw

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@volcano-sh-bot volcano-sh-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 1, 2019
@volcano-sh-bot volcano-sh-bot merged commit 5f271fa into volcano-sh:master Nov 1, 2019
@@ -91,6 +74,8 @@ func (pc *Scheduler) runOnce() {
defer glog.V(4).Infof("End scheduling ...")
defer metrics.UpdateE2eDuration(metrics.Duration(scheduleStartTime))

pc.loadSchedulerConf()
Copy link
Collaborator

Choose a reason for hiding this comment

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

For better performance, we can add a file watcher

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants