libflux/composite_future: propagate child errors to parent #2099
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Problem: Composite wait "any" or "all" futures are fulfilled with sucess even if one of the child futures that makes up the composite is fulfilled with an error. This makes it tricky to use the composites as a component in a flux_future_and_then() chain, since the error will not break the chain. This can cause unexpected failures from the next continuation in the chain. When fulfilling the dummy "parent" future for a composite, propagate an error if a child future was fulfilled with an error. This means that a composite "wait all" future is only fulfilled with success if all of its child futures are fulfilled with success, and that a "wait any" is fulfilled with success if the first future to complete is sucessful.
Add tests to the composite future unit tests to ensure that child futures in a composite that are fulfilled with an error propagate that error to the parent.
|
Restarted a builder that failed in |
|
This seems straighforward and has some good tests, so I'm for merging if you're ready. |
|
Yeah, pretty simple one so I think it is ready. Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is something I ran across in the
job-execmodule, when buildingflux_future_and_thenchains of composite futures.Currently, composite "wait all" and "wait any" futures are fulfilled with success as soon as all or any of their child futures are fulfilled, whether with success or failure. This makes it challenging to use
and_thenchains, which are supposed to break immediately (propagate the error) when an intermediate future is fulfilled with error.This PR just sets up the check for composite future fulfillment to propagate any error from the children to the "parent". To see the exact error or more detail, the user should still iterate over child futures (e.g. to see which future or futures were fulfilled with error)