Skip to content

http1: enable reads when final pipeline response received#6578

Merged
mattklein123 merged 2 commits intomasterfrom
fix_h1_fc_disconnect
Apr 15, 2019
Merged

http1: enable reads when final pipeline response received#6578
mattklein123 merged 2 commits intomasterfrom
fix_h1_fc_disconnect

Conversation

@mattklein123
Copy link
Member

Previously we were doing this when we create a new stream, but on
a reused connection this can lead to us missing an upstream
disconnection when the connection is placed back in the pool.

Fixes #6190

Risk Level: Medium. Scary stuff but well tested.
Testing: Modified existing UT.
Docs Changes: N/A
Release Notes: N/A

Previously we were doing this when we create a new stream, but on
a reused connection this can lead to us missing an upstream
disconnection when the connection is placed back in the pool.

Fixes #6190

Signed-off-by: Matt Klein <mklein@lyft.com>
Signed-off-by: Matt Klein <mklein@lyft.com>
@duderino
Copy link

duderino commented May 8, 2019

This is great. What's the story for http2?

@alyssawilk
Copy link
Contributor

H2 doesn't read disable, it marks the stream flow control blocked, and the used window is reclaimed on stream destruction:
https://github.com/envoyproxy/envoy/blob/master/source/common/http/http2/codec_impl.cc#L177

@iandyh
Copy link
Contributor

iandyh commented May 21, 2019

@duderino Hello. Which istio release we are expecting to include this fix? Thank you.

lambdai pushed a commit to lambdai/envoy-dai that referenced this pull request May 29, 2019
…#6578)

Previously we were doing this when we create a new stream, but on
a reused connection this can lead to us missing an upstream
disconnection when the connection is placed back in the pool.

Fixes envoyproxy#6190

Signed-off-by: Matt Klein <mklein@lyft.com>
@ldemailly
Copy link

reminiscing of #1895 and #2715 - hopefully this is the final fix ! 🥂

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.

Missed upstream disconnect leading to 503 UC

6 participants