Flush parent before reading on H2 connection #1557
Conversation
| @@ -172,6 +173,9 @@ private boolean tryConfigurePipeline() { | |||
| } | |||
|
|
|||
| pipeline.addLast(LastHttpContentHandler.create()); | |||
| if (Protocol.HTTP2.equals(protocol)) { | |||
| pipeline.addLast(FlushOnReadHandler.getInstance()); | |||
There was a problem hiding this comment.
Do we need to remove FlushOnReadHandler from the pipeline when we release the channel?
| ctx.channel().parent().flush(); | ||
| } | ||
|
|
||
| public static FlushOnReadHandler getInstance() { |
There was a problem hiding this comment.
create to be consistent with other static factory method?
There was a problem hiding this comment.
We have both :)
My preference is for getInstance
There was a problem hiding this comment.
Right, but we are using create in core consistently and I'd prefer using create going forward since this naming convention is written in our design doc. :)
There was a problem hiding this comment.
The key phrase is is "new instance" though. getInstance() makes it explicit that it's a singleton instance
Codecov Report
@@ Coverage Diff @@
## master #1557 +/- ##
============================================
+ Coverage 75.4% 75.47% +0.07%
Complexity 771 771
============================================
Files 884 884
Lines 27568 27625 +57
Branches 2155 2172 +17
============================================
+ Hits 20787 20850 +63
+ Misses 5786 5785 -1
+ Partials 995 990 -5
Continue to review full report at Codecov.
|
It's possible that WINDOW_UPDATE in the outbound buffer is not be written in a timely manner to the socket, causing subsequent read()'s to hang until the remote endpoint sees the local window increase.
dagnir
force-pushed
the
h2-flush-before-read
branch
from
d80ef4e to
219c46f
Compare
Description
It's possible that WINDOW_UPDATE in the outbound buffer is not be written in a
timely manner to the socket, causing subsequent read()'s to hang until the
remote endpoint sees the local window increase.
Motivation and Context
Testing
New unit and integ tests. Running full integ and benchmark suites.
Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsLicense