-
Notifications
You must be signed in to change notification settings - Fork 915
Fix async request cancellation #1112
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work.
80ffda9
to
2bfe7c7
Compare
CompletableFuture<AllTypesResponse> responseFuture = client.allTypes(r -> {}); | ||
responseFuture.cancel(true); | ||
Thread.sleep(500); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is 500 sufficient or will this cause transient failures?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, I'm just trying to figure out if this will fix the test fails in Travis ATM. I'll probably increase to 1s if it works
266f50e
to
d176350
Compare
Codecov Report
@@ Coverage Diff @@
## master #1112 +/- ##
============================================
+ Coverage 58.49% 58.52% +0.03%
- Complexity 4494 4507 +13
============================================
Files 737 738 +1
Lines 22663 22715 +52
Branches 1690 1698 +8
============================================
+ Hits 13256 13295 +39
- Misses 8718 8729 +11
- Partials 689 691 +2
Continue to review full report at Codecov.
|
- Correctly pass along the cancelled signal down to the async client if the future returned for an async operation is cancelled. - Don't wait for acquire to finish to cancel; instead fail the promise if not complete yet CancellableAcquireChannelPool added to take care of closing and releasing a cancelled acquire.
d176350
to
84714ac
Compare
…3363cff76 Pull request: release <- staging/0e6dd94a-3bd9-40aa-a91e-77f3363cff76
Description
Correctly pass along the cancelled signal down to the async client if the
future returned for an async operation is cancelled.
Don't wait for acquire to finish to cancel; instead fail the promise if not
complete yet
CancellableAcquireChannelPool added to take care of closing and releasing a
cancelled acquire.
Motivation and Context
Bug fix.
Testing
Added/updated tests in
codegen
,codegen-generated-classes-test
, andnetty-nio-client
.Screenshots (if appropriate)
Types of changes
Checklist
mvn install
succeedsLicense