Repository navigation
[IO_URING] Keep the completion of failed IORing operations - #401
Merged
Merged
Conversation
FranzBusch
requested review from
jrflat
and removed request for
glessard and
lorentey
October 5, 2026 18:58
When an operation failed, the blocking consume methods dropped its completion and only reported the `Errno`. The caller could not tell which request failed, since the `context` was lost. This breaks any user that matches completions to requests. This PR makes `blockingConsumeCompletions` pass the completion of a failed operation together with its error. Errors of the ring itself, such as `.timeout`, are still passed without a completion. Additionally, `blockingConsumeCompletion(timeout:)` now returns the completion of a failed operation instead of throwing. It only throws errors of the ring itself. I also added `Completion.error`, which is `nil` on success and the decoded `Errno` otherwise to make it easy to get to the error of a completion. Fixes #364 Fixes #395
FranzBusch
force-pushed
the
fb-iouring-completion-error
branch
from
October 5, 2026 18:59
2de2728 to
cbf57d4
Compare
jrflat
approved these changes
Oct 6, 2026
jrflat
left a comment
jrflat
left a comment
Contributor
There was a problem hiding this comment.
LGTM, I think it's a good behavior change but we should call it out in a release note for the next minor bump.
| /// (or forever if not specified). For each completed operation found, `consumer` is called to handle | ||
| /// processing it. | ||
| /// | ||
| /// `consumer` receives a completion, an error, and whether consuming is done: |
Contributor
There was a problem hiding this comment.
We should add a similar note to submitPreparedRequestsAndConsumeCompletions since this updates its behavior, too.
Member
Author
There was a problem hiding this comment.
Good catch. Updated and added a test
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
When an operation failed, the blocking consume methods dropped its completion and only reported the
Errno. The caller could not tell which request failed, since thecontextwas lost. This breaks any user that matches completions to requests.This PR makes
blockingConsumeCompletionspass the completion of a failed operation together with its error. Errors of the ring itself, such as.timeout, are still passed without a completion. Additionally,blockingConsumeCompletion(timeout:)now returns the completion of a failed operation instead of throwing. It only throws errors of the ring itself.I also added
Completion.error, which isnilon success and the decodedErrnootherwise to make it easy to get to the error of a completion.Fixes #364
Fixes #395