refactor:add the dependency probing logic of http and unify the apis of http1 and http2 #7684

YvCeung · 2025-10-09T12:03:39Z

I have read the CONTRIBUTING.md guidelines.
I have registered the PR changes.

Ⅰ. Describe what this PR did

1.Dynamically detects whether HTTP/1 or HTTP/2 is available in the target system, and gracefully falls back to HTTP/1 when HTTP/2 is unavailable.
2.Unifies the HTTP/1 and HTTP/2 API interfaces, allowing upper-level services to initiate HTTP requests simply through HttpExecutorFactory.getInstance().doGet(Post)

Ⅱ. Does this pull request fix one issue?

fix #7561
fix #7685

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

重构了 HTTP/1 和 HTTP/2 的服务能力，实现统一且易用的 API：

1.之前 HTTP/2 请求是异步的，需要调用方通过额外编写回调逻辑来处理响应，这对业务代码侵入性很强、改造量大，且与 HTTP/1 API 差异较大。本次通过引入 CompletableFuture 将异步调用转为同步调用，并统一返回值，使上层服务调用 HTTP/2 时可以像调用 HTTP/1 一样，无需编写回调逻辑，降低了调用方的改造成本。

HTTP/1 和 HTTP/2 的平滑切换逻辑封装在 HttpExecutorFactory 内，上层业务代码无需关注协议差异或手动判断是否支持 HTTP/2，保证了调用的简洁性和一致性。
本次针对HTTP/2的识别并未考虑到Server端的版本是否支持，所以在RaftRegistryServiceImpl 中做改造时(feature : Integrate Raft and Protocol Switching Based on Server Version #7586 )仍需要加入对Server端的判断

Refactored the HTTP/1 and HTTP/2 service capabilities to provide a unified and user-friendly API:

Previously, HTTP/2 requests were asynchronous and required the caller to handle callbacks. This approach was highly invasive to business code, required significant modifications, and was inconsistent with the HTTP/1 API. In this update, we use CompletableFuture to convert asynchronous calls into synchronous ones and standardize the return values, allowing upper-level services to call HTTP/2 just like HTTP/1, without writing callback logic, thus reducing the impact on the caller’s code.
The smooth fallback between HTTP/1 and HTTP/2 is fully encapsulated within HttpExecutorFactory. The business code does not need to worry about protocol differences or manually check for HTTP/2 support, ensuring simplicity and consistency in usage.
This update does not include detection of the server-side HTTP/2 support. Therefore, when refactoring RaftRegistryServiceImpl, it is still necessary to check whether the server supports HTTP/2.

common/src/main/java/org/apache/seata/common/http/Http2HttpExecutor.java

codecov · 2025-10-12T06:56:58Z

Codecov Report

❌ Patch coverage is 62.26415% with 80 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.46%. Comparing base (8763897) to head (3bb5013).

Files with missing lines	Patch %	Lines
...rg/apache/seata/common/http/Http2HttpExecutor.java	72.89%	19 Missing and 10 partials ⚠️
.../namingserver/NamingserverRegistryServiceImpl.java	0.00%	19 Missing ⚠️
...rg/apache/seata/common/http/Http1HttpExecutor.java	70.00%	7 Missing and 5 partials ⚠️
...scovery/registry/raft/RaftRegistryServiceImpl.java	20.00%	10 Missing and 2 partials ⚠️
.../java/org/apache/seata/common/http/HttpResult.java	73.33%	4 Missing ⚠️
...ache/seata/namingserver/manager/NamingManager.java	33.33%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##                2.x    #7684      +/-   ##
============================================
- Coverage     61.52%   61.46%   -0.07%     
- Complexity      680      684       +4     
============================================
  Files          1314     1316       +2     
  Lines         49890    49949      +59     
  Branches       5878     5890      +12     
============================================
+ Hits          30696    30702       +6     
- Misses        16404    16447      +43     
- Partials       2790     2800      +10

Files with missing lines	Coverage Δ
.../apache/seata/common/http/HttpExecutorFactory.java	`100.00% <100.00%> (ø)`
.../java/org/apache/seata/common/http/HttpResult.java	`73.33% <73.33%> (ø)`
...ache/seata/namingserver/manager/NamingManager.java	`75.53% <33.33%> (+4.82%)`	⬆️
...rg/apache/seata/common/http/Http1HttpExecutor.java	`63.10% <70.00%> (ø)`
...scovery/registry/raft/RaftRegistryServiceImpl.java	`22.96% <20.00%> (+0.08%)`	⬆️
.../namingserver/NamingserverRegistryServiceImpl.java	`31.54% <0.00%> (-12.13%)`	⬇️
...rg/apache/seata/common/http/Http2HttpExecutor.java	`72.89% <72.89%> (ø)`

... and 10 files with indirect coverage changes

🚀 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.

YvCeung · 2025-10-12T08:04:21Z

@YongGoose @funky-eyes PTAL

psxjoy

LGTM

YongGoose

The attempt at abstraction was good.

However, the way it’s actually used in the business logic feels a bit lacking.
Please let me know if I missed something.

YongGoose · 2025-10-15T13:06:13Z

common/src/main/java/org/apache/seata/common/http/Http1HttpExecutor.java

+    /**
+     * Gets instance.
+     *
+     * @return the instance
+     */
+    public static Http1HttpExecutor getInstance() {
+
+        if (instance == null) {
+            synchronized (Http1HttpExecutor.class) {
+                if (instance == null) {
+                    instance = new Http1HttpExecutor();
+                }
+            }
+        }
+        return instance;
+    }


Why does this instance need to be a singleton?

I think some resources may need to be reused here, such as the connection pool here, and using a singleton can avoid the state inconsistency problems caused by multiple instances and the performance problems caused by frequent object creation.

HTTP_CLIENT_MAP is already managed as a static final field.

Why does this instance need to be a singleton?

当我们希望通过 HTTP/2 发起请求时，不仅需要检测客户端是否支持，还必须确认 Seata Server 端是否具备 HTTP/2 能力。

如果直接使用抽象类提供的通用 API 发起调用，可能会出现这样的问题：客户端最终使用了 HTTP/2 实现类，但服务端并不支持，从而导致不可预期的异常。原因在于当前的依赖检测逻辑中，尚未包含对 Server 端协议版本的校验。

因此，为了保证在某些场景下能够安全运行，我暂时为每个实现类单独保留了 getInstance() 方法，便于手动选择合适的实现。

待后续在依赖探测中集成 “Server 端 HTTP/2 支持判断” 能力后，这些单例获取方法就可以被移除。届时，所有 HTTP 请求都可统一通过 HttpExecutorFactory.getInstance() 进行调用。

This logic serves as a transitional solution.

When we intend to initiate requests over HTTP/2, we must verify not only the client’s capability but also whether the Seata Server supports HTTP/2.

If we directly invoke the generic API provided by the abstract class, it’s possible that the actual implementation used on the client side is based on HTTP/2 while the server does not support it. This could lead to unpredictable exceptions, since the current dependency check mechanism does not include validation for the server-side protocol version.

To ensure stability under such conditions, each implementation class temporarily provides its own getInstance() method, allowing manual selection of the proper executor.

Once the capability detection logic is enhanced to determine whether the server supports HTTP/2, these per-class singleton access methods can be safely removed. At that point, all HTTP requests can be uniformly initiated via HttpExecutorFactory.getInstance().

YongGoose · 2025-10-15T13:07:29Z

...n/java/org/apache/seata/discovery/registry/namingserver/NamingserverRegistryServiceImpl.java

-            try (CloseableHttpResponse response = HttpClientUtil.doPost(url, jsonBody, header, 3000)) {
-                int statusCode = response.getStatusLine().getStatusCode();
+            try {
+                HttpResult httpResult = Http1HttpExecutor.getInstance().doPost(url, jsonBody, header, 3000);


It feels awkward to retrieve an object through the implementation’s getInstance() when we already have an abstraction.

It feels awkward to retrieve an object through the implementation’s getInstance() when we already have an abstraction.

It’s currently unclear whether Seata’s internal namingServer requires server-side version compatibility checks when operating over HTTP/2.

If such checks are unnecessary, this logic can be simplified to use the abstract class’s generic API directly.
However, if the server’s protocol version must still be validated, this issue essentially aligns with the scenario described above — meaning that server-side version detection should be integrated into the dependency checking mechanism.

YongGoose · 2025-10-15T13:09:21Z

common/src/main/java/org/apache/seata/common/http/HttpExecutorFactory.java

+    private static HttpExecutor createInstance() {
+        String implName = isOkHttpAvailable() ? HTTP1_IMPL : HTTP2_IMPL;
+        return EnhancedServiceLoader.load(HttpExecutor.class, implName);
+    }
+
+    private static boolean isOkHttpAvailable() {
+        try {
+            Class.forName("okhttp3.OkHttpClient", false, HttpExecutorFactory.class.getClassLoader());
+            return true;
+        } catch (ClassNotFoundException ignored) {
+            LOGGER.warn("HTTP2 implementation not available in classpath, falling back to default executor!");
+            return false;
+        }
+    }


What is it used for?

What is it used for?

Checks whether the client includes the OkHttp3 dependency to determine if it has the capability to send HTTP/2 requests.

@YongGoose In your PR(#7586 ), would it be possible to integrate the server-side version check(isHttp2Supported) into the current dependency detection logic?
If so, I think you could refactor it after my PR gets merged — that way, the per-implementation singleton methods would no longer be necessary.

YvCeung and others added 5 commits October 9, 2025 18:38

init structure

2dafe93

unify api

dd105cd

modify http1 api

9996b4e

refactor clusterControllerTest

10410e7

Compatible with historical code

70f08c4

YvCeung marked this pull request as ready for review October 9, 2025 12:28

ZhangYv and others added 19 commits October 9, 2025 20:59

unify http2 api

d42e3cd

optimize code format

3a6bbbc

optimize unit test case

2e880b4

optimize http2 unit test case

208a2ab

modify test case

1625bfe

fix ut case

f3113a9

format code

0dc5686

fix some ut case

ae8efe4

fix some ut case

5b91c51

Merge branch '2.x' into xiaoyu1009

eadde63

optimize code

67a4ffc

format code

f8af053

fix error ut case

63d2189

optimize code

7ade817

optimize code

a54b947

Remove duplicate apis

8ecb132

Supplement the implementation logic of the api

5b791f4

Simplify the api of http2

01e44d1

format code

fb2efb4

github-advanced-security bot found potential problems Oct 12, 2025

View reviewed changes

common/src/main/java/org/apache/seata/common/http/Http2HttpExecutor.java Show resolved Hide resolved

add ut case

7037727

YvCeung changed the title ~~【WIP】Add the dependency probing logic of http and unify the apis of http1 and http2~~ Add the dependency probing logic of http and unify the apis of http1 and http2 Oct 12, 2025

add changes

2a39bde

YvCeung changed the title ~~Add the dependency probing logic of http and unify the apis of http1 and http2~~ Refactor:add the dependency probing logic of http and unify the apis of http1 and http2 Oct 12, 2025

YvCeung changed the title ~~Refactor:add the dependency probing logic of http and unify the apis of http1 and http2~~ refactor:add the dependency probing logic of http and unify the apis of http1 and http2 Oct 12, 2025

YvCeung mentioned this pull request Oct 13, 2025

Http5ClientUtilTest.testDoPostHttp_body_onSuccess_forceHttp1:250 expected: <true> but was: <false> #7685

Open

2 tasks

psxjoy approved these changes Oct 13, 2025

View reviewed changes

YongGoose self-requested a review October 13, 2025 23:06

YvCeung and others added 4 commits October 15, 2025 18:09

remove chinese doc

e15b3af

Merge branch '2.x' into xiaoyu1009

6089844

modify url build case

0777d6d

Merge remote-tracking branch 'origin/xiaoyu1009' into xiaoyu1009

3bb5013

YongGoose requested changes Oct 15, 2025

View reviewed changes

YvCeung added 3 commits October 16, 2025 14:24

Merge branch '2.x' into xiaoyu1009

93b77c0

Merge branch '2.x' into xiaoyu1009

9ca1711

Merge branch '2.x' into xiaoyu1009

568b7f0

refactor:add the dependency probing logic of http and unify the apis of http1 and http2 #7684

Are you sure you want to change the base?

refactor:add the dependency probing logic of http and unify the apis of http1 and http2 #7684

Conversation

YvCeung commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

Uh oh!

Uh oh!

codecov bot commented Oct 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

YvCeung commented Oct 12, 2025

Uh oh!

psxjoy left a comment

Choose a reason for hiding this comment

Uh oh!

YongGoose left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

YvCeung commented Oct 9, 2025 •

edited

Loading

codecov bot commented Oct 12, 2025 •

edited

Loading