[chore](http) Merge ms and recycler http skeleton by wyxxxcat · Pull Request #61502 · apache/doris

wyxxxcat · 2026-03-19T02:52:06Z

What problem does this PR solve?

Problem Summary: Meta service and recycler maintained duplicated HTTP parsing, routing, and config handling logic, which made it harder to reuse handlers and keep versioned endpoints consistent. This change extracts the shared HTTP helper layer into cloud/src/common/ http_helper.*, introduces role-based handler registration for MetaService and Recycler, reuses common config show/update helpers, and adds versioned cluster HTTP test coverage.

If you want to expand the interface version

{"add_cluster",
 {.handler = [](void* s, brpc::Controller* c) { return process_alter_cluster((MS*)s, c); },
  .versioned_handlers =
          {{"v2",
            [](void* s, brpc::Controller* c) {
                return process_alter_cluster_v2((MS*)s, c);
            }}},
  .role = HttpRole::META_SERVICE}},

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

Release note

None

Check List (For Author)

Test
- Regression test
- Unit Test
- Manual test (add detailed scripts or steps below)
- No need to test or manual test. Explain why:
  - This is a refactor/code format and no logic has been changed.
  - Previous test can cover this change.
  - No code files have been changed.
  - Other reason
Behavior changed:
- No.
- Yes.
Does this need documentation?
- No.
- Yes.

Check List (For Reviewer who merge this PR)

Confirm the release note
Confirm test cases
Confirm document
Add branch pick label

Thearas · 2026-03-19T02:52:12Z

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

What problem was fixed (it's best to include specific error reporting information). How it was fixed.
Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
What features were added. Why was this function added?
Which code was refactored and why was this part of the code refactored?
Which functions were optimized and what is the difference before and after the optimization?

wyxxxcat · 2026-03-19T03:06:54Z

run buildall

wyxxxcat · 2026-03-24T02:20:17Z

run buildall

wyxxxcat · 2026-04-01T07:36:12Z

run buildall

wyxxxcat · 2026-04-01T09:03:05Z

run buildall

hello-stephen · 2026-04-01T09:29:40Z

Cloud UT Coverage Report

Increment line coverage 60.24% (615/1021) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	77.91% (1834/2354)
Line Coverage	64.36% (32465/50440)
Region Coverage	65.09% (16221/24919)
Branch Coverage	55.58% (8627/15522)

wyxxxcat · 2026-04-02T02:03:39Z

run buildall

hello-stephen · 2026-04-02T02:43:04Z

Cloud UT Coverage Report

Increment line coverage 60.24% (615/1021) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	77.91% (1834/2354)
Line Coverage	64.36% (32462/50440)
Region Coverage	65.07% (16214/24919)
Branch Coverage	55.59% (8629/15522)

wyxxxcat · 2026-04-14T08:56:13Z

run buildall

hello-stephen · 2026-04-14T10:02:57Z

Cloud UT Coverage Report

Increment line coverage 59.69% (659/1104) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	77.96% (1839/2359)
Line Coverage	64.53% (32862/50923)
Region Coverage	65.15% (16306/25028)
Branch Coverage	55.67% (8692/15614)

github-actions · 2026-04-14T10:35:28Z

PR approved by at least one committer and no changes requested.

github-actions · 2026-04-14T10:35:31Z

PR approved by anyone and no changes requested.

w41ter · 2026-04-15T08:25:15Z

/review

wyxxxcat · 2026-04-16T02:22:09Z

/review

github-actions

I found one blocking regression in the extracted HTTP helper.

Critical checkpoint conclusions:

Goal / correctness: Partially met. The shared HTTP/router extraction mostly preserves existing routes, but show_config no longer guarantees valid JSON output for all config values.
Minimality / focus: The change is focused on HTTP helper consolidation, although it is mechanically large.
Concurrency / locks: No new lock-order or thread-safety issue was identified in the dispatcher/helper path.
Lifecycle / static init: The new function-local static handler tables look safe; I did not find a cross-TU static initialization issue.
Config / dynamic behavior: No new config items were added, but the shared config show/update path regressed response correctness for some legitimate string values.
Compatibility: Route compatibility is mostly preserved, including legacy v1 MetaService paths. The show_config response is no longer backward-compatible once a string value contains JSON-special characters.
Parallel paths: MetaService and Recycler now share config handlers; that shared path is where the regression was introduced.
Conditional checks: The route-version and role checks are understandable and acceptable.
Test coverage: Version-routing tests were added, but there is still no coverage for show_config on string configs containing JSON-special characters, which would have caught this regression.
Observability: Existing logging/status propagation is mostly preserved.
Transaction / persistence: The reviewed change does not materially alter transaction/MoW/recycler core flows; the cloud module-specific checkpoints there appear unaffected.
Data writes / atomicity: update_config still delegates to the existing persistence/update logic; I did not find a new atomicity issue there.
FE-BE variable passing: Not applicable.
Performance: I did not identify a significant performance regression in the new routing path.
Other issues: None beyond the blocking show_config regression below.

Review scope note: this review was based on code inspection of the touched files; I did not run a build or test binary in the runner.

Please fix the inline issue and add coverage for the affected show_config path.

wyxxxcat · 2026-04-16T03:01:44Z

run buildall

hello-stephen · 2026-04-16T03:45:57Z

Cloud UT Coverage Report

Increment line coverage 61.11% (693/1134) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	78.02% (1842/2361)
Line Coverage	64.63% (32936/50958)
Region Coverage	65.25% (16335/25034)
Branch Coverage	55.79% (8712/15616)

### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: Keep cloud config::show_config using structured JSON serialization so string-valued configs with JSON-special characters remain valid when http_json_reply reparses the payload. ### Release note Fix /show_config so mutable string config values containing JSON-special characters are returned correctly. ### Check List (For Author) - Test: Regression test / Unit Test / Manual test / No need to test (with reason) - Unit Test: ./run-cloud-ut.sh --run --filter=meta_service_test:MetaServiceHttpTest.ShowConfigEscapesJsonSpecialCharacters -j 8 - Behavior changed: Yes (show_config now preserves valid JSON output for escaped string values) - Does this need documentation: No

wyxxxcat · 2026-04-17T09:57:07Z

run buildall

github-actions · 2026-04-17T10:00:54Z

PR approved by at least one committer and no changes requested.

hello-stephen · 2026-04-17T10:30:56Z

Cloud UT Coverage Report

Increment line coverage 61.11% (693/1134) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	78.02% (1842/2361)
Line Coverage	64.61% (32924/50958)
Region Coverage	65.23% (16329/25034)
Branch Coverage	55.78% (8710/15616)

gavinchou · 2026-04-19T08:42:36Z

/review

github-actions · 2026-04-19T10:38:41Z

OpenCode automated review failed and did not complete.

Error: Review step was failure (possibly timeout or cancelled)
Workflow run: https://github.com/apache/doris/actions/runs/24625077724

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

dismiss

### What problem does this PR solve? Problem Summary: Meta service and recycler maintained duplicated HTTP parsing, routing, and config handling logic, which made it harder to reuse handlers and keep versioned endpoints consistent. This change extracts the shared HTTP helper layer into `cloud/src/common/ http_helper.*`, introduces role-based handler registration for MetaService and Recycler, reuses common config show/update helpers, and adds versioned cluster HTTP test coverage. If you want to expand the interface version ``` {"add_cluster", {.handler = [](void* s, brpc::Controller* c) { return process_alter_cluster((MS*)s, c); }, .versioned_handlers = {{"v2", [](void* s, brpc::Controller* c) { return process_alter_cluster_v2((MS*)s, c); }}}, .role = HttpRole::META_SERVICE}}, ```

…62603) Cherry-picked from #61502 Co-authored-by: Yixuan Wang <wangyixuan@selectdb.com> Co-authored-by: Dongyang Li <lidongyang@selectdb.com>

wyxxxcat requested review from dataroaring, gavinchou and w41ter as code owners March 19, 2026 02:52

wyxxxcat force-pushed the chore_ms_recycler_http branch 2 times, most recently from f8e2b7e to 0521bc4 Compare March 24, 2026 02:04

wyxxxcat force-pushed the chore_ms_recycler_http branch from 0521bc4 to a9b5cc1 Compare April 1, 2026 07:32

wyxxxcat added dev/3.1.x dev/4.0.x dev/4.1.x labels Apr 1, 2026

wyxxxcat force-pushed the chore_ms_recycler_http branch from cd3c674 to efdcadd Compare April 2, 2026 02:01

w41ter reviewed Apr 10, 2026

View reviewed changes

Comment thread cloud/src/common/http_helper.cpp Outdated

Comment thread cloud/src/common/configbase.cpp Outdated

wyxxxcat force-pushed the chore_ms_recycler_http branch 3 times, most recently from 90872fb to 6e7be19 Compare April 14, 2026 08:33

gavinchou previously approved these changes Apr 14, 2026

View reviewed changes

github-actions Bot added the approved Indicates a PR has been approved by one committer. label Apr 14, 2026

github-actions Bot added the reviewed label Apr 14, 2026

wyxxxcat force-pushed the chore_ms_recycler_http branch from 6e7be19 to 9951f56 Compare April 16, 2026 02:21

github-actions Bot removed the approved Indicates a PR has been approved by one committer. label Apr 16, 2026

github-actions Bot previously requested changes Apr 16, 2026

View reviewed changes

Comment thread cloud/src/common/configbase.cpp Outdated

wyxxxcat added 2 commits April 17, 2026 17:57

1

749a3e5

wyxxxcat force-pushed the chore_ms_recycler_http branch from b8d9656 to cbfcbbf Compare April 17, 2026 09:57

gavinchou approved these changes Apr 17, 2026

View reviewed changes

github-actions Bot added the approved Indicates a PR has been approved by one committer. label Apr 17, 2026

gavinchou mentioned this pull request Apr 19, 2026

[fix](typo) fix typo 'sucess' -> 'success' typo in meta_service_http response #62539

Closed

1 task

hello-stephen merged commit 0d771c1 into apache:master Apr 19, 2026
30 of 31 checks passed

github-actions Bot added the dev/4.0.x-conflict label Apr 19, 2026

github-actions Bot mentioned this pull request Apr 19, 2026

branch-4.1: [chore](http) Merge ms and recycler http skeleton #61502 #62603

Merged

gavinchou removed dev/4.0.x dev/4.0.x-conflict labels Apr 19, 2026

mymeiyi mentioned this pull request Apr 21, 2026

[fix](wal) Fix WAL replay loading 0 rows due to columns being marked as missing #62658

Merged

yiguolei added dev/4.1.1-merged and removed dev/4.1.x labels Apr 23, 2026

wyxxxcat added int/3.1.6-merged and removed dev/3.1.x labels May 7, 2026

Conversation

wyxxxcat commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

Release note

Check List (For Author)

Check List (For Reviewer who merge this PR)

Uh oh!

Thearas commented Mar 19, 2026

Uh oh!

wyxxxcat commented Mar 19, 2026

Uh oh!

wyxxxcat commented Mar 24, 2026

Uh oh!

wyxxxcat commented Apr 1, 2026

Uh oh!

wyxxxcat commented Apr 1, 2026

Uh oh!

hello-stephen commented Apr 1, 2026

Cloud UT Coverage Report

Uh oh!

wyxxxcat commented Apr 2, 2026

Uh oh!

hello-stephen commented Apr 2, 2026

Cloud UT Coverage Report

Uh oh!

Uh oh!

Uh oh!

wyxxxcat commented Apr 14, 2026

Uh oh!

hello-stephen commented Apr 14, 2026

Cloud UT Coverage Report

Uh oh!

github-actions Bot commented Apr 14, 2026

Uh oh!

github-actions Bot commented Apr 14, 2026

Uh oh!

w41ter commented Apr 15, 2026

Uh oh!

wyxxxcat commented Apr 16, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wyxxxcat commented Apr 16, 2026

Uh oh!

hello-stephen commented Apr 16, 2026

Cloud UT Coverage Report

Uh oh!

wyxxxcat commented Apr 17, 2026

Uh oh!

github-actions Bot commented Apr 17, 2026

Uh oh!

hello-stephen commented Apr 17, 2026

Cloud UT Coverage Report

Uh oh!

gavinchou commented Apr 19, 2026

Uh oh!

github-actions Bot commented Apr 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

wyxxxcat commented Mar 19, 2026 •

edited

Loading