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

Support WriteAsync cancellation token #1645

Merged
merged 5 commits into from
Apr 6, 2022

Conversation

JamesNK
Copy link
Member

@JamesNK JamesNK commented Mar 21, 2022

Fixes #1422

Turns out it was pretty complicated, especially retries and hedging. PR adds support in all places:

  • WriteAsync cancellation on server
  • WriteAsync cancellation on client
  • WriteAsync cancellation on client for retries and hedging

Also fixed a bug when hedging and writing a large message to a client stream which causes the call to be committed because it exceeded buffer size. I found this while testing.

Also fixed a bug where the server would silently continue if it was aborted while writing a message. Now throws OperationCanceledException.

Note: Contains some changes from #1644 because Grpc.Core.Api needed to be updated.

@JamesNK JamesNK force-pushed the jamesnk/writeasync-cancellation branch 2 times, most recently from 8e0b246 to 99dac4d Compare March 22, 2022 22:12
@JamesNK JamesNK force-pushed the jamesnk/writeasync-cancellation branch from d2ce394 to 6e4bec5 Compare March 23, 2022 05:31
@JamesNK
Copy link
Member Author

JamesNK commented Mar 25, 2022

The longer I look at this PR, the more bugs I find in other places. New bug fixes:

  • Client streaming hedge calls wait for WriteAsync to succeed at least once before continuing.
  • Buffered hedge calls are explicitly flushed.
  • Fix race between server FlushAsync exiting and request cancellation token reporting cancellation.

I want to get this merged before it grows any more.

@JamesNK JamesNK merged commit bfe312a into grpc:master Apr 6, 2022
@JamesNK JamesNK deleted the jamesnk/writeasync-cancellation branch April 6, 2022 00:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add IAsyncStreamWriter<T>.WriteAsync(T, CancellationToken) overload
2 participants