Start informers after leader election

[linxiulei]

If scheduler fails to be elected, it should not start informers so that to avoid memory overhead.

Signed-off-by: Eric Lin exlin@google.com

What type of PR is this?

/kind bug

What this PR does / why we need it:

Improve kube-scheduler memory efficiency

Which issue(s) this PR fixes:

Fixes #115753

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Added new config option `delayCacheUntilActive` to `KubeSchedulerConfiguration` that can provide a tradeoff between memory efficiency and scheduling speed when their leadership is updated in `kube-scheduler`

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[k8s-ci-robot]

Hi @linxiulei. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[aojea]

/ok-to-test
/lgtm
/assign @alculquicondor

[alculquicondor]

I'm not convinced this is a bug.

Having the caches ready to go implies that when a scheduler goes down, a second one immediately picks up.

Not sure who take this decision initially or if a pro-con analysis was performed. Invoking @wojtek-t as he was involved in #45453

[aojea]

Having the caches ready to go implies that when a scheduler goes down, a second one immediately picks up.

/lgtm cancel

that is an good point indeed

[linxiulei]

Not sure who take this decision initially or if a pro-con analysis was performed. Invoking @wojtek-t as he was involved in #45453

I investigated it a bit. As far as I could see for other components (eg. KCM), they only start caching after leader election. Maybe scheduler is very latency sensitive but IMHO, I'd prefer memory efficiency than scheduling latency here.

[alculquicondor]

I'd prefer memory efficiency than scheduling latency here.

Sounds like it should be an option. Not everyone would be happy with this.

[kerthcet]

I think quick failover is important for HA, so prefer to keep it as it-is. I would not take it a bug.
/remove-kind bug
/kind cleanup ?

[k8s-ci-robot]

@kerthcet: The label(s) kind/? cannot be applied, because the repository doesn't have them.

In response to this:

I think quick failover is important for HA, so prefer to keep the change. I would not take it a bug.
/remove-kind bug
/kind cleanup ?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[liggitt]

one suggestion on the config field doc to avoid double negatives, lgtm otherwise

[liggitt]

/lgtm
/approve

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: d298ea32aaebb28f0a3c101898b2e3a14b8cffa4

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, liggitt, linxiulei

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:

• cmd/kube-scheduler/OWNERS [alculquicondor,liggitt]
• pkg/generated/openapi/OWNERS [liggitt]
• pkg/scheduler/apis/config/OWNERS [alculquicondor,liggitt]
• staging/src/k8s.io/kube-scheduler/config/OWNERS [liggitt]

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

[liggitt]

/lgtm

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: ac721a233a7833ce7415e588e8dced9891d5b677

[everpeace]

@alculquicondor

This PR introduces a new config API in KubeSchedulerConfiguration that can provide a tradeoff between memory efficiency and scheduling speed when their leadership is updated in kube-scheduler. I think this PR should have a release note item. WDYT?

[alculquicondor]

oh, that is correct. It's adding an API field, so it's user visible.

@linxiulei please add a release note.

[linxiulei]

Done. Thanks @everpeace & @alculquicondor

[alculquicondor]

nit:

/release-note-edit

Added new config option `delayCacheUntilActive` to `KubeSchedulerConfiguration` that can provide a tradeoff between memory efficiency and scheduling speed when their leadership is updated in `kube-scheduler`

we use the yaml/json name in documentation