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

Incorrect transmission error handling in redundant transport #222

Closed
pavel-kirienko opened this issue Apr 23, 2022 · 0 comments · Fixed by #223
Closed

Incorrect transmission error handling in redundant transport #222

pavel-kirienko opened this issue Apr 23, 2022 · 0 comments · Fixed by #223
Assignees

Comments

@pavel-kirienko
Copy link
Member

# Execute the work items concurrently and unblock as soon as the first inferior is done transmitting.
# Those that are still pending are cancelled because we're not going to wait around for the slow ones.
done, pending = await asyncio.wait(futures, return_when=asyncio.FIRST_COMPLETED)
assert len(done) + len(pending) == len(inferiors) == len(futures)
_logger.debug("%s send results: done=%s, pending=%s", self, done, pending)
for p in pending:
p.cancel() # We will no longer need this.
# Extract the results to determine the final outcome of the transaction.
results = [x.result() for x in done if x.exception() is None]
exceptions = [x.exception() for x in done if x.exception() is not None]

If there are two inferiors and one of them gets blocked transiently to complete the transmission while the other one raises an exception, the former's future is detached to complete in the background and then the only remaining result is the error, which is then propagated to the caller.

This is incorrect because errors should not unblock the transmission.

The correct behavior is to wait until either the first inferior is done transmitting or all of them have failed.

The behavior described in the method documentation is actually correct but the implementation is not compliant.

An example with a larger context is available here: https://ci.appveyor.com/project/Zubax/yakut/build/job/6k9185femyoi8qq7#L2708

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant