[opt](brpc) check and remove unavailable brpc stubs by mrhhsg · Pull Request #43212 · apache/doris

mrhhsg · 2024-11-04T09:12:04Z

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

Check List (For Committer)

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 colde files have been changed.
  - Other reason
Behavior changed:
- No.
- Yes.
Does this need documentation?
- No.
- Yes.
Release note

None

Check List (For Reviewer who merge this PR)

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

doris-robot · 2024-11-04T09:12:10Z

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

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

github-actions · 2024-11-04T09:16:55Z

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

yiguolei · 2024-11-04T09:24:45Z

+void FragmentMgr::register_brpc_with_query(const TNetworkAddress& network_address,
+                                           std::shared_ptr<PBackendService_Stub> brpc_stub,
+                                           TUniqueId query_id) {
+    std::lock_guard<std::mutex> lock(_lock);


the lock will very very busy, we could save it in query context and collect it during cancel worker

github-actions

clang-tidy made some suggestions

yiguolei · 2024-11-04T10:37:24Z

            c->clear_finished_tasks();
        }

+        std::unordered_map<TNetworkAddress, BrpcItem> brpc_stub_with_queries;


不要这么写，会出现2个rpc 链接的host 一样，但是brpc的对象不一样的问题。
可以使用brpc stub的指针做key，然后把network address 放到BrpcItem 这里面，供check的时候打log 使用。

yiguolei · 2024-11-04T10:59:31Z

+        if (cntl.Failed()) {
+            error_message = cntl.ErrorText();
+            LOG(WARNING) << "brpc stub: " << network_address << " check failed: " << error_message;
+        } else if (response.has_status() && response.has_hello() && response.hello() == message &&


Do not need check response value.
If has response, it means we could connect dest succesfully, then the connection is OK.

yiguolei · 2024-11-04T11:09:19Z

+        request.set_hello(message);
+        PHandShakeResponse response;
+        brpc::Controller cntl;
+        cntl.set_timeout_ms(500);


500 * (failed_count + 1) * 2

yiguolei · 2024-11-04T11:14:22Z

 private:
+    struct BrpcItem {
+        std::shared_ptr<PBackendService_Stub> brpc_stub;
+        std::map<std::string, std::weak_ptr<QueryContext>> queries;


why it is a map? a vector is not ok?

github-actions · 2024-11-05T13:30:31Z

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

github-actions · 2024-11-05T13:30:33Z

PR approved by anyone and no changes requested.

yiguolei · 2024-11-05T13:35:30Z

run buildall

doris-robot · 2024-11-05T15:24:53Z

TeamCity be ut coverage result:
Function Coverage: 37.86% (9838/25986)
Line Coverage: 29.03% (81841/281923)
Region Coverage: 28.26% (42164/149188)
Branch Coverage: 24.85% (21400/86118)
Coverage Report: http://coverage.selectdb-in.cc/coverage/7b29bb3e90eb3951f30bcce3d779d8ab2464e178_7b29bb3e90eb3951f30bcce3d779d8ab2464e178/report/index.html

mrhhsg · 2024-11-06T01:55:19Z

run buildall

doris-robot · 2024-11-06T03:49:58Z

TeamCity be ut coverage result:
Function Coverage: 37.85% (9837/25986)
Line Coverage: 29.03% (81836/281926)
Region Coverage: 28.26% (42163/149191)
Branch Coverage: 24.84% (21396/86120)
Coverage Report: http://coverage.selectdb-in.cc/coverage/efc91f5511d83fc1a0ec9916528d660fe20cc7de_efc91f5511d83fc1a0ec9916528d660fe20cc7de/report/index.html

mrhhsg · 2024-11-11T07:55:30Z

run buildall

mrhhsg · 2024-11-11T08:39:40Z

run buildall

doris-robot · 2024-11-11T09:54:24Z

TeamCity be ut coverage result:
Function Coverage: 37.77% (9843/26058)
Line Coverage: 29.05% (82294/283279)
Region Coverage: 28.18% (42314/150158)
Branch Coverage: 24.76% (21453/86634)
Coverage Report: http://coverage.selectdb-in.cc/coverage/950a530b549508dbc2dbd45bab02e350a5c57c9e_950a530b549508dbc2dbd45bab02e350a5c57c9e/report/index.html

mrhhsg · 2024-11-12T03:22:04Z

run buildall

yiguolei · 2024-11-12T06:58:54Z

@@ -912,9 +915,18 @@ void FragmentMgr::cancel_worker() {
                        LOG_WARNING("Query {} is timeout", print_id(it->first));
                        queries_timeout.push_back(it->first);
                        ++it;


这个it ++ 是多余的

github-actions · 2024-11-12T07:31:39Z

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

yiguolei · 2024-11-12T07:42:32Z

run buildall

xinyiZzz

LGTM

yiguolei reviewed Nov 4, 2024

View reviewed changes

mrhhsg force-pushed the opt_brpc branch 3 times, most recently from c7ce8e0 to 02163a0 Compare November 4, 2024 10:26

github-actions bot reviewed Nov 4, 2024

View reviewed changes

Comment thread be/src/runtime/query_context.h

yiguolei reviewed Nov 4, 2024

View reviewed changes

Comment thread be/src/runtime/fragment_mgr.cpp

yiguolei reviewed Nov 4, 2024

View reviewed changes

mrhhsg force-pushed the opt_brpc branch from 02163a0 to 7b29bb3 Compare November 4, 2024 16:05

yiguolei previously approved these changes Nov 5, 2024

View reviewed changes

github-actions bot added the approved Indicates a PR has been approved by one committer. label Nov 5, 2024

github-actions bot added the reviewed label Nov 5, 2024

mrhhsg dismissed yiguolei’s stale review via efc91f5 November 6, 2024 01:55

mrhhsg force-pushed the opt_brpc branch from 7b29bb3 to efc91f5 Compare November 6, 2024 01:55

github-actions bot removed the approved Indicates a PR has been approved by one committer. label Nov 6, 2024

mrhhsg force-pushed the opt_brpc branch from efc91f5 to d6a1c00 Compare November 11, 2024 07:55

mrhhsg force-pushed the opt_brpc branch from d6a1c00 to 950a530 Compare November 11, 2024 08:39

[opt](brpc) check and remove unavailable brpc stubs

dd67b9d

mrhhsg force-pushed the opt_brpc branch from 5328be3 to dd67b9d Compare November 12, 2024 03:21

yiguolei reviewed Nov 12, 2024

View reviewed changes

fix code

4d4be6b

yiguolei approved these changes Nov 12, 2024

View reviewed changes

github-actions bot added the approved Indicates a PR has been approved by one committer. label Nov 12, 2024

yiguolei approved these changes Nov 12, 2024

View reviewed changes

yiguolei added the dev/2.1.x label Nov 12, 2024

xinyiZzz approved these changes Nov 13, 2024

View reviewed changes

yiguolei merged commit 47dab4d into apache:master Nov 13, 2024

github-actions bot added the cherry-pick-conflict-in-2.1.x label Nov 13, 2024

mrhhsg deleted the opt_brpc branch November 13, 2024 06:50

mrhhsg added a commit to mrhhsg/doris that referenced this pull request Nov 13, 2024

[opt](brpc) check and remove unavailable brpc stubs (apache#43212)

6bbc716

mrhhsg mentioned this pull request Nov 13, 2024

[opt](brpc) check and remove unavailable brpc stubs (#43212) #43859

Merged

16 tasks

mrhhsg added a commit to mrhhsg/doris that referenced this pull request Nov 13, 2024

[opt](brpc) check and remove unavailable brpc stubs (apache#43212)

3adf314

yiguolei pushed a commit to mrhhsg/doris that referenced this pull request Nov 14, 2024

[opt](brpc) check and remove unavailable brpc stubs (apache#43212)

7fe5738

yiguolei pushed a commit that referenced this pull request Nov 14, 2024

[opt](brpc) check and remove unavailable brpc stubs (#43212) (#43859)

7fc78e3

yiguolei added dev/2.1.8-merged and removed dev/2.1.x cherry-pick-conflict-in-2.1.x labels Nov 14, 2024

yiguolei added the dev/3.0.x label Dec 2, 2024

github-actions bot added the dev/3.0.x-conflict label Dec 2, 2024

mrhhsg added a commit to mrhhsg/doris that referenced this pull request Dec 2, 2024

[opt](brpc) check and remove unavailable brpc stubs (apache#43212)

912ebfc

mrhhsg mentioned this pull request Dec 2, 2024

[opt](brpc) check and remove unavailable brpc stubs (#43212) #44840

Closed

16 tasks

mrhhsg added a commit to mrhhsg/doris that referenced this pull request Dec 2, 2024

[opt](brpc) check and remove unavailable brpc stubs (apache#43212)

80f5c0f

yiguolei removed dev/3.0.x dev/3.0.x-conflict labels Jun 24, 2025

Conversation

mrhhsg commented Nov 4, 2024

What problem does this PR solve?

Check List (For Committer)

Check List (For Reviewer who merge this PR)

Uh oh!

doris-robot commented Nov 4, 2024

Uh oh!

github-actions bot commented Nov 4, 2024

Uh oh!

yiguolei Nov 4, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yiguolei Nov 4, 2024

Choose a reason for hiding this comment

Uh oh!

yiguolei Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yiguolei Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yiguolei Nov 4, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 5, 2024

Uh oh!

github-actions bot commented Nov 5, 2024

Uh oh!

yiguolei commented Nov 5, 2024

Uh oh!

doris-robot commented Nov 5, 2024

Uh oh!

mrhhsg commented Nov 6, 2024

Uh oh!

doris-robot commented Nov 6, 2024

Uh oh!

mrhhsg commented Nov 11, 2024

Uh oh!

mrhhsg commented Nov 11, 2024

Uh oh!

doris-robot commented Nov 11, 2024

Uh oh!

mrhhsg commented Nov 12, 2024

Uh oh!

yiguolei Nov 12, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 12, 2024

Uh oh!

yiguolei commented Nov 12, 2024

Uh oh!

xinyiZzz left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yiguolei Nov 4, 2024 •

edited

Loading

yiguolei Nov 4, 2024 •

edited

Loading