Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[feature-wip](BE http)Support BE http service with brpc #16123

Merged
merged 20 commits into from
Feb 28, 2023
Merged

[feature-wip](BE http)Support BE http service with brpc #16123

merged 20 commits into from
Feb 28, 2023

Conversation

TangSiyang2001
Copy link
Collaborator

@TangSiyang2001 TangSiyang2001 commented Jan 20, 2023 •
edited
Loading

Proposed changes

Issue Number: close #16087

Problem summary

Doris BE uses libevent to support http service currently, which is planned to be replaced by brpc step by step in the future.

Doris BE has webserver_port for http service and brpc_port for internal service. The brpc-based http service will share the brpc_port with internal service, and webserver_port will be kept for libevent-based http service at the current stage.

Checklist(Required)

  1. Does it affect the original behavior:
    • Yes
    • No
    • I don't know
  2. Has unit tests been added:
    • Yes
    • No
    • No Need
  3. Has document been added or modified:
    • Yes
    • No
    • No Need
  4. Does it need to update dependencies:
    • Yes
    • No
  5. Are there any changes that cannot be rolled back:
    • Yes (If Yes, please explain WHY)
    • No

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@TangSiyang2001 TangSiyang2001 changed the title [enhancement-wip] Support BE http service with brpc [enhancement-wip](BE http)Support BE http service with brpc Jan 20, 2023
github-actions[bot]
github-actions bot reviewed Jan 20, 2023
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@hello-stephen
Copy link
Contributor

hello-stephen commented Jan 20, 2023 •
edited
Loading

TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 34.44 seconds
stream load tsv: 478 seconds loaded 74807831229 Bytes, about 149 MB/s
stream load json: 40 seconds loaded 2358488459 Bytes, about 56 MB/s
stream load orc: 68 seconds loaded 1101869774 Bytes, about 15 MB/s
stream load parquet: 29 seconds loaded 861443392 Bytes, about 28 MB/s
https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20230225044928_clickbench_pr_104305.html

@TangSiyang2001 TangSiyang2001 force-pushed the brpc-http branch 5 times, most recently from e4ffeeb to c6e1cc6 Compare January 21, 2023 07:17
github-actions[bot]
github-actions bot reviewed Jan 21, 2023
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@TangSiyang2001 TangSiyang2001 force-pushed the brpc-http branch 3 times, most recently from 553f028 to e41692e Compare January 21, 2023 11:58
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

github-actions[bot]
github-actions bot reviewed Jan 22, 2023
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@TangSiyang2001 TangSiyang2001 force-pushed the brpc-http branch 4 times, most recently from 9233657 to ccc9e36 Compare January 23, 2023 01:55
@morningman
Copy link
Contributor

Is there a way to keep http port unchanged but using brpc implemented http server?

@TangSiyang2001 TangSiyang2001 force-pushed the brpc-http branch 2 times, most recently from 6f3873e to 92720a8 Compare January 25, 2023 09:38
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@TangSiyang2001
Copy link
Collaborator Author

TangSiyang2001 commented Jan 25, 2023 •
edited
Loading

Is there a way to keep http port unchanged but using brpc implemented http server?

Yes, doris has now got a brpc port for internal service and has established a brpc server. Because brpc server supports several services ,we can add the new http service to this server to share the brpc port, and the original http port is completely unaffected.

@TangSiyang2001 TangSiyang2001 force-pushed the brpc-http branch 2 times, most recently from 432c51e to b9e7a1a Compare January 25, 2023 10:21
github-actions[bot]
github-actions bot reviewed Feb 24, 2023
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

github-actions[bot]
github-actions bot reviewed Feb 24, 2023
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@TangSiyang2001
Copy link
Collaborator Author

run compile

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@Yukang-Lian Yukang-Lian mentioned this pull request Feb 27, 2023
Open
dataroaring
dataroaring approved these changes Feb 28, 2023
View reviewed changes
Copy link
Contributor

@dataroaring dataroaring left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Feb 28, 2023
@github-actions
Copy link
Contributor

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

@dataroaring dataroaring merged commit 049eccc into apache:master Feb 28, 2023
@github-actions
Copy link
Contributor

PR approved by anyone and no changes requested.

dataroaring added a commit to dataroaring/incubator-doris that referenced this pull request Feb 28, 2023
@dataroaring
Revert "[feature-wip](BE http)Support BE http service using brpc (apa…
1520750 
…che#16123)"

This reverts commit 049eccc.

Merge back after streamload is handled.
@TangSiyang2001 TangSiyang2001 mentioned this pull request Feb 28, 2023
Closed
5 tasks
morningman pushed a commit that referenced this pull request Mar 1, 2023
@dataroaring
Revert "[feature-wip](BE http)Support BE http service using brpc (#16123
e687f3b 
)" (#17219)

This reverts commit 049eccc.
Merge back after streamload is handled.
HappenLee added a commit to HappenLee/incubator-doris that referenced this pull request Mar 7, 2023
@HappenLee
Revert "[feature-wip](BE http)Support BE http service using brpc (apa…
9281a2a 
…che#16123)"

This reverts commit 049eccc.
yagagagaga pushed a commit to yagagagaga/doris that referenced this pull request Mar 9, 2023
@TangSiyang2001 @yagagagaga
[feature-wip](BE http)Support BE http service using brpc (apache#16123)
1ed0231 
Now, streamload is not supported.
yagagagaga pushed a commit to yagagagaga/doris that referenced this pull request Mar 9, 2023
@dataroaring @yagagagaga
Revert "[feature-wip](BE http)Support BE http service using brpc (apa…
bf7b80f 
…che#16123)" (apache#17219)

This reverts commit 049eccc.
Merge back after streamload is handled.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@dataroaring dataroaring dataroaring approved these changes

@platoneko platoneko Awaiting requested review from platoneko

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by one committer. kind/test reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Enhancement] Support BE http service with brpc.
7 participants
@TangSiyang2001 @hello-stephen @morningman @yangzhg @yongjinhou @platoneko @dataroaring