Skip to content

Conversation

@zrlw
Copy link
Contributor

@zrlw zrlw commented Jun 9, 2025

What is the purpose of the change?

submit pr again for #15368 which has been reverted because it has defects should be fixed: connectionPrefaceReceivedPromise should be created before calling performConnect, otherwise no promise could be set if server http2settings frame was received before this preparation.

try to fix #15233 No provider available issue, the roots of the issue is client sent large headers frame before receiving server connection preface (http2 settings frame), which cause server sent back go away frame and the connection was disconnected by the server.
go away issue
see details at https://httpwg.org/specs/rfc7540.html#ConnectionHeader

3.5. HTTP/2 Connection Preface
In HTTP/2, each endpoint is required to send a connection preface as a final confirmation of the protocol in use and to establish the initial settings for the HTTP/2 connection. The client and server each send a different connection preface.
The client connection preface starts with a sequence of 24 octets, which in hex notation is:
0x505249202a20485454502f322e300d0a0d0a534d0d0a0d0a
That is, the connection preface starts with the string PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n). This sequence MUST be followed by a SETTINGS frame (Section 6.5), which MAY be empty. 
... ...
The server connection preface consists of a potentially empty SETTINGS frame (Section 6.5) that MUST be the first frame the server sends in the HTTP/2 connection.

https://quafoo.gitbooks.io/http2-rfc7540-zh-cn-en/content/chapter3/section3.5.html

HTTP/2 Connection Preface / HTTP/2连接前奏
在HTTP/2中,要求两端都要发送一个连接前奏,作为对所使用协议的最终确认,并确定HTTP/2连接的初始设置。客户端和服务端各自发送不同的连接前奏。
客户端连接前奏以一个24字节的序列开始,用十六进制表示为:
0x505249202a20485454502f322e300d0a0d0a534d0d0a0d0a
即,连接前奏以字符串"PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"开始。这个序列后面必须跟一个可以为空的 SETTINGS 帧( 6.5节 )。
... ...
服务端连接前奏包含一个可能为空的 SETTINGS 帧( 6.5节 ),它必须由服务端在HTTP/2连接中首先发送。

Checklist

  • Make sure there is a GitHub_issue field for the change.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit-test to verify your logic correction. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
  • Make sure gitHub actions can pass. Why the workflow is failing and how to fix it?

@zrlw
Copy link
Contributor Author

zrlw commented Jun 9, 2025

@oxsean @AlbumenJ PTAL

@codecov-commenter
Copy link

codecov-commenter commented Jun 9, 2025

Codecov Report

❌ Patch coverage is 60.46512% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.91%. Comparing base (6f110c1) to head (1509ae9).
⚠️ Report is 88 commits behind head on 3.3.

Files with missing lines Patch % Lines
...moting/transport/netty4/NettyConnectionClient.java 51.61% 14 Missing and 1 partial ⚠️
...sport/netty4/http2/Http2ClientSettingsHandler.java 83.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                3.3   #15436      +/-   ##
============================================
+ Coverage     60.90%   60.91%   +0.01%     
- Complexity    11423    11440      +17     
============================================
  Files          1887     1888       +1     
  Lines         86266    86309      +43     
  Branches      12929    12933       +4     
============================================
+ Hits          52540    52576      +36     
- Misses        28290    28291       +1     
- Partials       5436     5442       +6     
Flag Coverage Δ
integration-tests-java17 33.09% <46.51%> (+0.07%) ⬆️
integration-tests-java8 33.14% <46.51%> (-0.05%) ⬇️
samples-tests-java17 31.46% <48.83%> (+0.03%) ⬆️
samples-tests-java8 29.26% <41.86%> (-0.11%) ⬇️
unit-tests 58.83% <48.83%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

protected void doConnect() throws RemotingException {
long start = System.currentTimeMillis();
super.doConnect();
if (connectionPrefaceReceivedPromiseRef == null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please extract this logic into a separate method, and in the comment, reference RFC7540#section-3.5 to explain why we need this.

@oxsean
Copy link
Contributor

oxsean commented Jun 9, 2025

LGTM.

@zrlw zrlw merged commit c31164a into apache:3.3 Jun 9, 2025
29 checks passed
@zrlw zrlw deleted the 3.3-optimized-addConnectionPreface4Http2 branch June 9, 2025 15:51
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.

[Bug] No provider available due to receiving go away frame immediately after successful connection

3 participants