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.

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

libflux/composite_future: propagate child errors to parent #2099

Merged
merged 2 commits into from Mar 25, 2019

Conversation

@grondo
Copy link
Contributor

commented Mar 22, 2019

This is something I ran across in the job-exec module, when building flux_future_and_then chains 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_then chains, 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)

grondo added 2 commits Mar 22, 2019
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.
@grondo

This comment has been minimized.

Copy link
Contributor Author

commented Mar 23, 2019

Restarted a builder that failed in t1004-kvs-namespace.t with what looked like a grace timeout (Killed by signal 9)

@garlick

This comment has been minimized.

Copy link
Member

commented Mar 24, 2019

This seems straighforward and has some good tests, so I'm for merging if you're ready.

@grondo

This comment has been minimized.

Copy link
Contributor Author

commented Mar 25, 2019

Yeah, pretty simple one so I think it is ready. Thanks!

@garlick garlick merged commit 6833978 into flux-framework:master Mar 25, 2019
2 checks passed
2 checks passed
Mergify — Summary 1 potential rule
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@grondo grondo deleted the grondo:composite-future-errors branch Jul 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.