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.

Sign up for GitHub

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

Jump to bottom

fix: Avoid controller crashes when running large number of workflows #9691

Merged
merged 3 commits into from
Sep 27, 2022
Merged

fix: Avoid controller crashes when running large number of workflows #9691

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dd8f6be
fix: Add limit to list calls
terrytangyuan Sep 26, 2022
9569212
chore: Address comments
terrytangyuan Sep 27, 2022
5131c67
fix: spelling
terrytangyuan Sep 27, 2022
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
1 change: 1 addition & 0 deletions .spelling
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ k8s-jobs
kube
kubelet
kubernetes
liveness
localhost
memoization
memoized
Expand Down
2 changes: 2 additions & 0 deletions docs/environment-variables.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ most users. Environment variables may be removed at any time.
| `GZIP_IMPLEMENTATION` | `string` | `PGZip` | The implementation of compression/decompression. Currently only "`PGZip`" and "`GZip`" are supported. |
| `INFORMER_WRITE_BACK` | `bool` | `true` | Whether to write back to informer instead of catching up. |
| `HEALTHZ_AGE` | `time.Duration` | `5m` | How old a un-reconciled workflow is to report unhealthy. |
| `HEALTHZ_LIST_LIMIT` | `int` | `200` | The maximum number of responses to return for a list call on workflows for liveness check. |
| `INDEX_WORKFLOW_SEMAPHORE_KEYS` | `bool` | `true` | Whether or not to index semaphores. |
| `LEADER_ELECTION_IDENTITY` | `string` | Controller's `metadata.name` | The ID used for workflow controllers to elect a leader. |
| `LEADER_ELECTION_DISABLE` | `bool` | `false` | Whether leader election should be disabled. |
| `LEADER_ELECTION_LEASE_DURATION` | `time.Duration` | `15s` | The duration that non-leader candidates will wait to force acquire leadership. |
| `LEADER_ELECTION_RENEW_DEADLINE` | `time.Duration` | `10s` | The duration that the acting master will retry refreshing leadership before giving up. |
| `LEADER_ELECTION_RETRY_PERIOD` | `time.Duration` | `5s` | The duration that the leader election clients should wait between tries of actions. |
| `LIST_LIMIT` | `int` | `200` | The maximum number of responses to return for a list call on workflows for workflow informer. |
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be removed, right?

| `MAX_OPERATION_TIME` | `time.Duration` | `30s` | The maximum time a workflow operation is allowed to run for before re-queuing the workflow onto the work queue. |
| `OFFLOAD_NODE_STATUS_TTL` | `time.Duration` | `5m` | The TTL to delete the offloaded node status. Currently only used for testing. |
| `POD_NAMES` | `string` | `v2` | Whether to have pod names contain the template name (v2) or be the node id (v1) - should be set the same for Argo Server. |
Expand Down
1 change: 1 addition & 0 deletions workflow/controller/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -803,6 +803,7 @@ func (wfc *WorkflowController) tweakListOptions(options *metav1.ListOptions) {
labelSelector := labels.NewSelector().
Add(util.InstanceIDRequirement(wfc.Config.InstanceID))
options.LabelSelector = labelSelector.String()
options.Limit = int64(env.LookupEnvIntOr("LIST_LIMIT", 200))
Copy link
Member

Choose a reason for hiding this comment

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

Will this paginate properly? Or will it simply cut it off at 200

}

func getWfPriority(obj interface{}) (int32, time.Time) {
Expand Down
5 changes: 3 additions & 2 deletions workflow/controller/healthz.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import (
)

var (
age = env.LookupEnvDurationOr("HEALTHZ_AGE", 5*time.Minute)
age = env.LookupEnvDurationOr("HEALTHZ_AGE", 5*time.Minute)
limit = int64(env.LookupEnvIntOr("HEALTHZ_LIST_LIMIT", 200))
)

// https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/#define-a-liveness-http-request
Expand All @@ -30,7 +31,7 @@ func (wfc *WorkflowController) Healthz(w http.ResponseWriter, r *http.Request) {
labelSelector := "!" + common.LabelKeyPhase + "," + instanceIDSelector
err := func() error {
// avoid problems with informers, but directly querying the API
list, err := wfc.wfclientset.ArgoprojV1alpha1().Workflows(wfc.managedNamespace).List(ctx, metav1.ListOptions{LabelSelector: labelSelector})
list, err := wfc.wfclientset.ArgoprojV1alpha1().Workflows(wfc.managedNamespace).List(ctx, metav1.ListOptions{LabelSelector: labelSelector, Limit: limit})
if err != nil {
return err
}
Expand Down