-
Notifications
You must be signed in to change notification settings - Fork 63
Added default admin config for MaxParallelism #262
Conversation
Signed-off-by: Prafulla Mahindrakar <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #262 +/- ##
==========================================
+ Coverage 57.47% 57.55% +0.08%
==========================================
Files 141 143 +2
Lines 10481 10553 +72
==========================================
+ Hits 6024 6074 +50
- Misses 3787 3809 +22
Partials 670 670
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Maybe get one more from Katrina?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we set a sensible default here: https://github.com/flyteorg/flyteadmin/blob/master/pkg/runtime/application_config_provider.go#L39
@kumare3 what sounds good? 100?
@katrogan its already set to 25 which was discussed in the slack chat . Let me know if we need a different default value |
@@ -37,7 +37,9 @@ var flyteAdminConfig = config.MustRegisterSection(flyteAdmin, &interfaces.Applic | |||
MetadataStoragePrefix: []string{"metadata", "admin"}, | |||
EventVersion: 1, | |||
AsyncEventsBufferSize: 100, | |||
MaxParallelism: 25, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh thanks! i missed this earlier
Signed-off-by: Prafulla Mahindrakar <[email protected]>
Signed-off-by: Prafulla Mahindrakar [email protected]
TL;DR
Workflow executions can be gated by the max number of parallel nodes that can be processed. Specific override support for this MaxParallelism value already exists but not a generic default in the FlyteAdmin config.
This PR adds support in the admin config and provides a default value of 25
Following is the order in which property override is checked
Type
Are all requirements met?
Complete description
How did you fix the bug, make the feature etc. Link to any design docs etc
Tracking Issue
flyteorg/flyte#1532
Follow-up issue
NA