From f2e05a46c353da24ad2ab1dbc678f2b85540a333 Mon Sep 17 00:00:00 2001 From: MichaelMorris Date: Thu, 6 Jun 2024 16:17:44 +0100 Subject: [PATCH 1/3] Add HIP for running hooks in parallel Signed-off-by: MichaelMorris --- hips/hip-0019.md | 68 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 hips/hip-0019.md diff --git a/hips/hip-0019.md b/hips/hip-0019.md new file mode 100644 index 00000000..924c72e4 --- /dev/null +++ b/hips/hip-0019.md @@ -0,0 +1,68 @@ +--- +hip: 0019 +title: "Enable Running Hooks in Parallel" +authors: [ "Michael Morris " ] +created: "2024-06-06" +type: "feature" +status: "draft" +--- + +## Abstract + +Helm hooks are currently executed serially. In many cases there will be no reason why they cannot be run in parallel. Running hooks in parallel can significantly improve install/upgrade times + +## Motivation + +Decrease install/upgrade times by executing hooks in parallel + +## Specification + +Add a new flag to the install, upgrade, rollback and uninstall commands to enable parallel behavior for hook execution. The flag will be of integer type and will allow the user to specify the maximum number of hooks that can be executed in parallel. + +The flag shall be optional. If not set or set to "0" or "1", hook execution shall be done serially. If set to a value greater than "1", hook execution shall be done in parallel, subject to not exceeding the given value in the number of hooks executed in parallel. + +Only hooks of the same weight shall be executed in parallel. + +## Rationale + +While it may be desirable to run hooks in parallel, particularly those of the same weight, doing so by default may cause issues for users depending on the existing serial behavior. Therefore it is proposed to keep the existing serial behavior by default and enable parallel behavior through the use of a flag. + +Making the flag an integer rather than boolean allows the user to possibility to restrict the number of parallel processes where resource constraints are a concern. + +Allowing hooks of different weights to be executed in parallel is probably not consistent with the documented behavior of lower weight hooks being executed before higher weighted hooks and, therefore only hooks of the same weight shall be executed in parallel + +## Backwards compatibility + +Proposed solution is fully backwards compatible with no changes in default behavior. + +## Security implications + +None + +## How to teach this + +Document in command line help and https://helm.sh/docs/topics/charts_hooks/. + +## Reference implementation + +PR submitted some time ago: https://github.com/helm/helm/pull/11804 +May need to be updated depending on decisions taken on what is proposed here. + +## Rejected ideas + +Why certain ideas that were brought while discussing this HIP were not +ultimately pursued. + +## Open issues + +|Issue |Proposal|Decision| +|-----------------------------------------------------------------------------------------|--------|--------| +|Should we allow hooks to be run in parallel |Yes | | +|Should the default behaviour continue to be to execute in serial |Yes | | +|Is there benefit in allowing the user limit the max number of hooks executing in parallel|Yes | | +|Should only hooks of the same weight be executed in parallel |Yes | | + + +## References + +There exists an older closed HIP on hook parallelism, but its focus was on test hooks: https://github.com/helm/community/pull/165 \ No newline at end of file From 425ba4881a382cc987ce8a7be2c5c0601d71dec1 Mon Sep 17 00:00:00 2001 From: MichaelMorris Date: Fri, 21 Jun 2024 11:27:27 +0100 Subject: [PATCH 2/3] Refactored to control via chart metadata instead of flag Signed-off-by: MichaelMorris --- hips/hip-0019.md | 155 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 143 insertions(+), 12 deletions(-) diff --git a/hips/hip-0019.md b/hips/hip-0019.md index 924c72e4..4476a919 100644 --- a/hips/hip-0019.md +++ b/hips/hip-0019.md @@ -17,19 +17,151 @@ Decrease install/upgrade times by executing hooks in parallel ## Specification -Add a new flag to the install, upgrade, rollback and uninstall commands to enable parallel behavior for hook execution. The flag will be of integer type and will allow the user to specify the maximum number of hooks that can be executed in parallel. +Only hooks of the same weight may be executed in parallel. The chart designer can specify if hooks will be run in parallel. I am proposing two options, one more fine grained that the other. + +Option 1. +Add a new parameter to the chart metadata of type boolean with name 'runHooksInParallel'. +When set to 'false' (default), the behavior will be as today (hooks run in sequence). +When set to 'true', hooks of the same weight for the chart shall be run in parallel. +In the case of subcharts, hooks of the same weight from different sub charts with 'runHooksInParallel' set to 'true' will be run in parallel. Hooks from subcharts with 'runHooksInParallel' to 'false' shall be run sequentially after hooks of the same weight that are permitted to run in parallel. + +Option 2. +Add a new parameter to the chart metadata with name 'runHooksInParallel' that accepts three values - 'false', 'otherChartsOnly' and 'true'. +Behavior for 'true' and 'false' is as per option 1. above. +The value 'otherChartsOnly' is only relevant in the case of subcharts and shall indicate that the hooks belonging to the particular subchart have to be run sequentially but, it is ok for this to happen in parallel with the execution of hooks from other subcharts + +For example, given a chart with subcharts as follows: + +|Sub chart|Hook|Weight| +|---------|----|------| +|chart A |H1 |0 | +| |H2 |1 | +|chart B |H3 |0 | +| |H4 |0 | +| |H5 |1 | +|chart C |H6 |0 | +| |H7 |1 | + + +With 'runHooksInParallel' set as follows: + +|chart |runHooksInParallel| +|-------|------------------| +|chart A|true | +|chart B|true | +|chart C|true | + +the execution order of the hooks shall be (hooks in same column are in parallel): + + + + + + + + + + + + + + + + + + + + + + + + + + +
Order of Execution
-------------->-----------
H1H2
H3H5
H4H7
H6
+ +i.e. Hooks H1, H3, H4 and H6 all having weight 0 are run in parallel, followed by hooks H2, H5 and H7 (all of which have weight 1) + +With 'runHooksInParallel' set as follows: + +|chart |runHooksInParallel| +|-------|------------------| +|chart A|true | +|chart B|false | +|chart C|true | + +the execution order of the hooks shall be: + + + + + + + + + + + + + + + + + + + + + + +
Order of Execution
---->----->----->---->---
H1H3H4H2H5
H6H7
+ +i.e. H3 and H4 are now run sequentially after H1 and H6 because the subchart they belong to has runHooksInParallel set to 'false'. Similarly H5 is now run after H2 and H7 + +With 'runHooksInParallel' set as follows: + +|chart |runHooksInParallel| +|-------|------------------| +|chart A|true | +|chart B|otherChartsOnly | +|chart C|true | + +the execution order of the hooks shall be: + + + + + + + + + + + + + + + + + + + + + + + + +
Order of Execution
---->------>----->----
H1H2H5
H3H4H7
H6
+ +i.e. as runHooksInParallel is now set to 'otherChartsOnly' for chart B, H3 and H4 are still run sequentially but in parallel with H1 and H6 which are from different subcharts -The flag shall be optional. If not set or set to "0" or "1", hook execution shall be done serially. If set to a value greater than "1", hook execution shall be done in parallel, subject to not exceeding the given value in the number of hooks executed in parallel. - -Only hooks of the same weight shall be executed in parallel. ## Rationale -While it may be desirable to run hooks in parallel, particularly those of the same weight, doing so by default may cause issues for users depending on the existing serial behavior. Therefore it is proposed to keep the existing serial behavior by default and enable parallel behavior through the use of a flag. +While it may be desirable to run hooks in parallel, particularly those of the same weight, doing so by default may cause issues for users depending on the existing serial behavior. Therefore it is proposed to keep the existing serial behavior by default and enable parallel behavior through the chart definition. -Making the flag an integer rather than boolean allows the user to possibility to restrict the number of parallel processes where resource constraints are a concern. +Allowing hooks of different weights to be executed in parallel is probably not consistent with the documented behavior of lower weight hooks being executed before higher weighted hooks and, therefore only hooks of the same weight shall be executed in parallel. -Allowing hooks of different weights to be executed in parallel is probably not consistent with the documented behavior of lower weight hooks being executed before higher weighted hooks and, therefore only hooks of the same weight shall be executed in parallel +Option 2 makes the logic more complex, but allows the chart designer to specify the hooks for a subchart must be run sequentially without blocking the hooks for other subcharts where they are of no significance. ## Backwards compatibility @@ -50,18 +182,17 @@ May need to be updated depending on decisions taken on what is proposed here. ## Rejected ideas -Why certain ideas that were brought while discussing this HIP were not -ultimately pursued. +Add a flag to control parallel behavior. +It is the chart developer that will have the knowledge of the application to determine if hooks can be run in parallel rather than the chart installer. Therefore, control should lie in the chart itself rather than the install (etc.) commands ## Open issues |Issue |Proposal|Decision| |-----------------------------------------------------------------------------------------|--------|--------| |Should we allow hooks to be run in parallel |Yes | | -|Should the default behaviour continue to be to execute in serial |Yes | | -|Is there benefit in allowing the user limit the max number of hooks executing in parallel|Yes | | +|Should the default behavior continue to be to execute in serial |Yes | | |Should only hooks of the same weight be executed in parallel |Yes | | - +|Is boolean value ok or should we allow more granularity, i.e. option1 or option 2 | | ## References From 304850ef3887047f701b70fb1c10605014e5eeb2 Mon Sep 17 00:00:00 2001 From: MichaelMorris Date: Fri, 21 Jun 2024 17:48:25 +0100 Subject: [PATCH 3/3] Added chart example Signed-off-by: MichaelMorris --- hips/hip-0019.md | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/hips/hip-0019.md b/hips/hip-0019.md index 4476a919..ca4e0deb 100644 --- a/hips/hip-0019.md +++ b/hips/hip-0019.md @@ -43,7 +43,7 @@ For example, given a chart with subcharts as follows: | |H7 |1 | -With 'runHooksInParallel' set as follows: +* With 'runHooksInParallel' set as follows: |chart |runHooksInParallel| |-------|------------------| @@ -82,12 +82,12 @@ the execution order of the hooks shall be (hooks in same column are in parallel) i.e. Hooks H1, H3, H4 and H6 all having weight 0 are run in parallel, followed by hooks H2, H5 and H7 (all of which have weight 1) -With 'runHooksInParallel' set as follows: +* With 'runHooksInParallel' set as follows: |chart |runHooksInParallel| |-------|------------------| |chart A|true | -|chart B|false | +|chart B|** false ** | |chart C|true | the execution order of the hooks shall be: @@ -117,17 +117,17 @@ the execution order of the hooks shall be: i.e. H3 and H4 are now run sequentially after H1 and H6 because the subchart they belong to has runHooksInParallel set to 'false'. Similarly H5 is now run after H2 and H7 -With 'runHooksInParallel' set as follows: +* With 'runHooksInParallel' set as follows: -|chart |runHooksInParallel| -|-------|------------------| -|chart A|true | -|chart B|otherChartsOnly | -|chart C|true | +|chart |runHooksInParallel | +|-------|----------------------| +|chart A|true | +|chart B|** otherChartsOnly ** | +|chart C|true | the execution order of the hooks shall be: - +
@@ -155,6 +155,16 @@ the execution order of the hooks shall be: i.e. as runHooksInParallel is now set to 'otherChartsOnly' for chart B, H3 and H4 are still run sequentially but in parallel with H1 and H6 which are from different subcharts +### Example Chart + + apiVersion: v1 + kind: Pod + metadata: + name: example-chart + runHooksInParallel: true + spec: + ... + ## Rationale While it may be desirable to run hooks in parallel, particularly those of the same weight, doing so by default may cause issues for users depending on the existing serial behavior. Therefore it is proposed to keep the existing serial behavior by default and enable parallel behavior through the chart definition.
Order of Execution