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

feat(controller): Make MAX_OPERATION_TIME configurable. Close #4239 #4562

Merged
merged 6 commits into from
Nov 21, 2020
Merged

feat(controller): Make MAX_OPERATION_TIME configurable. Close #4239 #4562

merged 6 commits into from
Nov 21, 2020
+44 −2

Conversation

alexec
Copy link
Contributor

@alexec alexec commented Nov 19, 2020 •
edited
Loading

Signed-off-by: Alex Collins [email protected]

Checklist:

This change is to test to see if increasing this improve issues like #4560

@alexec alexec linked an issue Nov 19, 2020 that may be closed by this pull request
Increase max reconciliation time #4239
Closed
@alexec alexec marked this pull request as ready for review November 20, 2020 19:01
alexec
alexec commented Nov 20, 2020
View reviewed changes
workflow/controller/operator.go Outdated
@@ -116,8 +116,21 @@ var (

// maxOperationTime is the maximum time a workflow operation is allowed to run
// for before requeuing the workflow onto the workqueue.
const maxOperationTime = 10 * time.Second
const defaultRequeueTime = maxOperationTime
var maxOperationTime = 10 * time.Second
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jessesuen @sarabala1979 this change allows you to configure the maxOperationTime. I've confirm this with a user that higher values (30s tested) prevent zombie worklows. We increase the number of workflow workers from 8 to 32 back in May. I think we should set this to 20s by default. Thoughts?

@alexec alexec changed the title feat(controller): Make MAX_OPERATION_TIME configurable. feat(controller): Make MAX_OPERATION_TIME configurable. Close #4239 Nov 20, 2020
@alexec alexec added this to the v2.12 milestone Nov 20, 2020
@alexec
Copy link
Contributor Author

alexec commented Nov 20, 2020

@sarabala1979 @jessesuen I've increased defaulte maxOperationTime (and defaultRequeueTime) to 30s

@alexec
mot: AM util/env/env.go
a4d7f49 
Signed-off-by: Alex Collins <[email protected]>
sarabala1979
sarabala1979 reviewed Nov 20, 2020
View reviewed changes
workflow/controller/operator.go
const maxOperationTime = 10 * time.Second
const defaultRequeueTime = maxOperationTime
var (
maxOperationTime = envutil.LookupEnvDurationOr("MAX_OPERATION_TIME", 30*time.Second)
Copy link
Member

Choose a reason for hiding this comment

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

I am fine with 30s because one of our ML customers is going really big workflow with 2000 + dynamic steps. 30s will help this workflow parse the big JSON parameters. It is configurable so if there is any backlog issue, we configure it back to 10s or 20s

@alexec alexec merged commit 15fd579 into argoproj:master Nov 21, 2020
@alexec alexec deleted the mot branch November 21, 2020 01:01
alexec added a commit that referenced this pull request Nov 21, 2020
@alexec
feat(controller): Make MAX_OPERATION_TIME configurable. Close #4239 (#…
e8ba1ed 
…4562)

Signed-off-by: Alex Collins <[email protected]>
brabster pushed a commit to brabster/argo that referenced this pull request Nov 24, 2020
@alexec @brabster
feat(controller): Make MAX_OPERATION_TIME configurable. Close argopro…
bf38cb2 
…j#4239 (argoproj#4562)

Signed-off-by: Alex Collins <[email protected]>
Signed-off-by: Paul Brabban <[email protected]>
alexcapras pushed a commit to alexcapras/argo that referenced this pull request Dec 2, 2020
@secretlifeof @alexcapras
docs: Clean-up examples. Fixes argoproj#4124 (argoproj#4128)
d133355 
Signed-off-by: [email protected] <[email protected]>

feat(ui): Add Template/Cron workflow filter to workflow page. Closes argoproj#4532 (argoproj#4543)

Signed-off-by: Tianchu Zhao <[email protected]>

feat(executor): Auto create s3 bucket if not present.

Signed-off-by: Alex Capras <[email protected]>

Apply codegen

Signed-off-by: Alex Capras <[email protected]>

Add argo-e2e label to test wf

Signed-off-by: Alex Capras <[email protected]>

chore: Updated stress test YAML (argoproj#4569)

Signed-off-by: Alex Collins <[email protected]>

docs: Updated kubectl apply command in manifests README (argoproj#4577)

Signed-off-by: Stefan Gloutnikov <[email protected]>

feat(controller): Make MAX_OPERATION_TIME configurable. Close argoproj#4239 (argoproj#4562)

Signed-off-by: Alex Collins <[email protected]>

docs: Fix a typo in example (argoproj#4590)

Signed-off-by: Takayoshi Nishida <[email protected]>

feat(controller): Retry transient offload errors. Resolves argoproj#4464 (argoproj#4482)

Signed-off-by: Alex Collins <[email protected]>

fix(server): use the correct name when downloading artifacts (argoproj#4579)

Signed-off-by: Daniel Herman <[email protected]>

fix(server): serve artifacts directly from disk to support large artifacts (argoproj#4589)

Signed-off-by: Daniel Herman <[email protected]>

fix(executor): Handle sidecar killing in a process-namespace-shared pod (argoproj#4575)

Signed-off-by: Daisuke Taniwaki <[email protected]>

docs: Add JSON schema for IDE validation (argoproj#4581)

Signed-off-by: Paul Brabban <[email protected]>

refactor: Use polling model for workflow phase metric (argoproj#4557)

Signed-off-by: Simon Behar <[email protected]>

Addressing reviewers comments

Signed-off-by: Alex Capras <[email protected]>

Addressing reviewers comments

docs: Minor typo fix (argoproj#4610)

Signed-off-by: Paavo Pokkinen <[email protected]>

fix(controller): Prevent tasks with names starting with digit to use either 'depends' or 'dependencies' (argoproj#4598)

Signed-off-by: terrytangyuan <[email protected]>

fix(docs): Bring minio chart instructions up to date (argoproj#4586)

Signed-off-by: Ranga Krishnan <[email protected]>

fix(executor): Fixed waitMainContainerStart returning prematurely. Closes argoproj#4599 (argoproj#4601)

Signed-off-by: fsiegmund <[email protected]>

feat(controller): Enhanced artifact repository ref. See argoproj#3184 (argoproj#4458)

Signed-off-by: Alex Collins <[email protected]>

fix: Null check pagination variable (argoproj#4617)

Signed-off-by: Simon Behar <[email protected]>

fix: Perform fields filtering server side (argoproj#4595)

Signed-off-by: Simon Behar <[email protected]>

fix(server): Correct webhook event payload marshalling. Fixes argoproj#4572 (argoproj#4594)

Signed-off-by: Alex Collins <[email protected]>

feat(ui): Add columns--narrower-height to AttributeRow (argoproj#4371)

fix: Fix TestCleanFieldsExclude (argoproj#4625)

Signed-off-by: Simon Behar <[email protected]>

fix(argo-server): fix global variable validation error with reversed dag.tasks (argoproj#4369)

Signed-off-by: chenyu.zheng <[email protected]>

fix: derive jsonschema and fix up issues, validate examples dir… (argoproj#4611)

Signed-off-by: Paul Brabban <[email protected]>

fix(ui): Reference secrets in EnvVars. Fixes argoproj#3973  (argoproj#4419)

Signed-off-by: Alejandro Tejera <[email protected]>

fix(ui): Fix Snyk issues (argoproj#4631)

Signed-off-by: Alex Collins <[email protected]>

feat(executor): More informative log when executors do not support output param from base image layer (argoproj#4620)

Signed-off-by: terrytangyuan <[email protected]>

Codegen patch. Signed off by [email protected]

Codegen patch. Signed off by [email protected]

Delete test.patch
alexcapras pushed a commit to alexcapras/argo that referenced this pull request Dec 2, 2020
@secretlifeof @alexcapras
docs: Clean-up examples. Fixes argoproj#4124 (argoproj#4128)
1a0cebb 
Signed-off-by: [email protected] <[email protected]>

feat(ui): Add Template/Cron workflow filter to workflow page. Closes argoproj#4532 (argoproj#4543)

Signed-off-by: Tianchu Zhao <[email protected]>

feat(executor): Auto create s3 bucket if not present.

Signed-off-by: Alex Capras <[email protected]>

Apply codegen

Signed-off-by: Alex Capras <[email protected]>

Add argo-e2e label to test wf

Signed-off-by: Alex Capras <[email protected]>

chore: Updated stress test YAML (argoproj#4569)

Signed-off-by: Alex Collins <[email protected]>

docs: Updated kubectl apply command in manifests README (argoproj#4577)

Signed-off-by: Stefan Gloutnikov <[email protected]>

feat(controller): Make MAX_OPERATION_TIME configurable. Close argoproj#4239 (argoproj#4562)

Signed-off-by: Alex Collins <[email protected]>

docs: Fix a typo in example (argoproj#4590)

Signed-off-by: Takayoshi Nishida <[email protected]>

feat(controller): Retry transient offload errors. Resolves argoproj#4464 (argoproj#4482)

Signed-off-by: Alex Collins <[email protected]>

fix(server): use the correct name when downloading artifacts (argoproj#4579)

Signed-off-by: Daniel Herman <[email protected]>

fix(server): serve artifacts directly from disk to support large artifacts (argoproj#4589)

Signed-off-by: Daniel Herman <[email protected]>

fix(executor): Handle sidecar killing in a process-namespace-shared pod (argoproj#4575)

Signed-off-by: Daisuke Taniwaki <[email protected]>

docs: Add JSON schema for IDE validation (argoproj#4581)

Signed-off-by: Paul Brabban <[email protected]>

refactor: Use polling model for workflow phase metric (argoproj#4557)

Signed-off-by: Simon Behar <[email protected]>

Addressing reviewers comments

Signed-off-by: Alex Capras <[email protected]>

Addressing reviewers comments

docs: Minor typo fix (argoproj#4610)

Signed-off-by: Paavo Pokkinen <[email protected]>

fix(controller): Prevent tasks with names starting with digit to use either 'depends' or 'dependencies' (argoproj#4598)

Signed-off-by: terrytangyuan <[email protected]>

fix(docs): Bring minio chart instructions up to date (argoproj#4586)

Signed-off-by: Ranga Krishnan <[email protected]>

fix(executor): Fixed waitMainContainerStart returning prematurely. Closes argoproj#4599 (argoproj#4601)

Signed-off-by: fsiegmund <[email protected]>

feat(controller): Enhanced artifact repository ref. See argoproj#3184 (argoproj#4458)

Signed-off-by: Alex Collins <[email protected]>

fix: Null check pagination variable (argoproj#4617)

Signed-off-by: Simon Behar <[email protected]>

fix: Perform fields filtering server side (argoproj#4595)

Signed-off-by: Simon Behar <[email protected]>

fix(server): Correct webhook event payload marshalling. Fixes argoproj#4572 (argoproj#4594)

Signed-off-by: Alex Collins <[email protected]>

feat(ui): Add columns--narrower-height to AttributeRow (argoproj#4371)

fix: Fix TestCleanFieldsExclude (argoproj#4625)

Signed-off-by: Simon Behar <[email protected]>

fix(argo-server): fix global variable validation error with reversed dag.tasks (argoproj#4369)

Signed-off-by: chenyu.zheng <[email protected]>

fix: derive jsonschema and fix up issues, validate examples dir… (argoproj#4611)

Signed-off-by: Paul Brabban <[email protected]>

fix(ui): Reference secrets in EnvVars. Fixes argoproj#3973  (argoproj#4419)

Signed-off-by: Alejandro Tejera <[email protected]>

fix(ui): Fix Snyk issues (argoproj#4631)

Signed-off-by: Alex Collins <[email protected]>

feat(executor): More informative log when executors do not support output param from base image layer (argoproj#4620)

Signed-off-by: terrytangyuan <[email protected]>

Codegen patch. Signed off by [email protected]

Codegen patch. Signed off by [email protected]

Delete test.patch

Signed-off-by: Alex Capras <[email protected]>
alexec added a commit that referenced this pull request Dec 9, 2020
@alexec
feat(controller): Make MAX_OPERATION_TIME configurable. Close #4239 (#…
2d05d56 
…4562)

Signed-off-by: Alex Collins <[email protected]>
@agilgur5 agilgur5 added the area/controller Controller issues, panics label Jul 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sarabala1979 sarabala1979 sarabala1979 approved these changes

Assignees

@sarabala1979 sarabala1979

Labels
area/controller Controller issues, panics
Projects
None yet
Milestone
v2.12
Development

Successfully merging this pull request may close these issues.

Increase max reconciliation time
3 participants
@alexec @sarabala1979 @agilgur5