[fix](brpc) Embed serialized request into the attachment and transmit it through http brpc by xinyiZzz · Pull Request #9803 · apache/doris

xinyiZzz · 2022-05-26T13:40:28Z

Proposed changes

Issue Number: close #7163

Problem Summary:

When the length of Tuple/Block data is greater than 2G, serialize the protoBuf request and embed the Tuple/Block data into the controller attachment and transmit it through http brpc.

This is to avoid errors when the length of the protoBuf request exceeds 2G: Bad request, error_text=[E1003]Fail to compress request.

In #7164, Tuple/Block data was put into attachment and sent via default baidu_std brpc, but when the attachment exceeds 2G, it will be truncated. There is no 2G limit for sending via http brpc.

Also, in #7921, consider putting Tuple/Block data into attachment transport by default, as this theoretically reduces one serialization and improves performance. However, the test found that the performance did not improve, but the memory peak increased due to the addition of a memory copy.

Upgrade:

The old request transfer attachement logic is expected to be removed in V1.3, and set transfer_large_data_by_brpc default true.

Therefore, the version before V1.3 needs to be upgraded to V1.3 before the upgrade can continue.

Checklist(Required)

Does it affect the original behavior: (Yes)
Has unit tests been added: (No)
Has document been added or modified: (Yes)
Does it need to update dependencies: (No)
Are there any changes that cannot be rolled back: (Yes)

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

yangzhg · 2022-05-27T02:24:42Z

    repeated string channels = 2;
 };

+message PEchoRequest {};


why named PEchoRequest ？ what dose echo mean

In the official example of brpc, use EchoRequest to represent empty Request.

brpc called EchoRequest because of it's service called echo
you can use PEmptyRequest or PEmptyTransmitDataParams

xinyiZzz · 2022-06-02T08:33:22Z

@yangzhg @morningman PTAL

morningman · 2022-06-03T09:35:34Z

+                                                 PTransmitDataResult* response,
+                                                 google::protobuf::Closure* done) {
+    SCOPED_SWITCH_BTHREAD();
+    PTransmitDataParams* request_raw = new PTransmitDataParams();


Memory leak?

In the new NewHttpClosure delete req and call the old done.run.

morningman

LGTM

github-actions · 2022-06-10T16:03:13Z

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

github-actions · 2022-06-10T16:03:16Z

PR approved by anyone and no changes requested.

yangzhg · 2022-06-11T00:05:57Z

PEchoRequest has not been renamed, should not merge it before resolve it

xinyiZzz · 2022-06-11T09:44:05Z

PEchoRequest has not been renamed, should not merge it before resolve it

fix, modified to PEmptyRequest , thks~

yangzhg

LGTM

github-actions · 2022-06-13T02:04:30Z

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

… it through http brpc (#9803) When the length of `Tuple/Block data` is greater than 2G, serialize the protoBuf request and embed the `Tuple/Block data` into the controller attachment and transmit it through http brpc. This is to avoid errors when the length of the protoBuf request exceeds 2G: `Bad request, error_text=[E1003]Fail to compress request`. In #7164, `Tuple/Block data` was put into attachment and sent via default `baidu_std brpc`, but when the attachment exceeds 2G, it will be truncated. There is no 2G limit for sending via `http brpc`. Also, in #7921, consider putting `Tuple/Block data` into attachment transport by default, as this theoretically reduces one serialization and improves performance. However, the test found that the performance did not improve, but the memory peak increased due to the addition of a memory copy.

github-actions Bot added area/vectorization kind/docs Categorizes issue or PR as related to documentation. labels May 26, 2022

xinyiZzz changed the title ~~brpc_http~~ [fix](brpc) Embed serialized request into the attachment and transmit it through http brpc May 26, 2022

xinyiZzz requested a review from yangzhg May 26, 2022 13:46

xinyiZzz force-pushed the brpc_http_push_v4 branch from d621af9 to 35a5bbb Compare May 26, 2022 13:49

morningman added this to the v1.2 milestone May 27, 2022

yangzhg reviewed May 27, 2022

View reviewed changes

Comment thread be/src/util/proto_util.h Outdated

yangzhg reviewed May 27, 2022

View reviewed changes

Comment thread be/src/common/config.h Outdated

yangzhg reviewed May 27, 2022

View reviewed changes

Comment thread be/src/exec/tablet_sink.cpp Outdated

morningman reviewed May 29, 2022

View reviewed changes

Comment thread be/src/util/proto_util.h Outdated

Comment thread be/src/common/config.h Outdated

Comment thread be/src/vec/sink/vdata_stream_sender.cpp Outdated

xinyiZzz force-pushed the brpc_http_push_v4 branch from 35a5bbb to 2d67c8b Compare June 1, 2022 18:35

morningman added the dev/1.0.1-deprecated should be merged into dev-1.0.1 branch label Jun 2, 2022

morningman modified the milestones: v1.2, v1.1 Jun 2, 2022

yangzhg reviewed Jun 3, 2022

View reviewed changes

Comment thread be/src/runtime/row_batch.cpp Outdated

morningman reviewed Jun 3, 2022

View reviewed changes

xinyiZzz force-pushed the brpc_http_push_v4 branch from 0356ec2 to 3145593 Compare June 8, 2022 14:10

yangzhg reviewed Jun 10, 2022

View reviewed changes

Comment thread be/src/util/brpc_client_cache.h Outdated

morningman previously approved these changes Jun 10, 2022

View reviewed changes

github-actions Bot added the approved Indicates a PR has been approved by one committer. label Jun 10, 2022

github-actions Bot added the reviewed label Jun 10, 2022

xinyiZzz dismissed morningman’s stale review via 6d50c54 June 11, 2022 09:42

github-actions Bot removed the approved Indicates a PR has been approved by one committer. label Jun 11, 2022

xinyiZzz force-pushed the brpc_http_push_v4 branch from 5d22d9a to 7735848 Compare June 11, 2022 17:31

xinyiZzz force-pushed the brpc_http_push_v4 branch from 7735848 to 21e8a38 Compare June 11, 2022 18:18

xinyiZzz added 9 commits June 12, 2022 22:49

brpc_http

0dc0150

2

1f0bcee

fix comment

56149ce

remove tupe data ptr

79362a4

fix push

2882ccc

fix format

c1b687d

fix reformat

13defed

fix commnet 0608

f7c365f

fix echo req

d30e9dc

xinyiZzz force-pushed the brpc_http_push_v4 branch from 21e8a38 to d30e9dc Compare June 12, 2022 15:11

yangzhg approved these changes Jun 13, 2022

View reviewed changes

github-actions Bot added the approved Indicates a PR has been approved by one committer. label Jun 13, 2022

morningman merged commit d58e00c into apache:master Jun 13, 2022

morningman added dev/merged-1.0.1-deprecated PR has been merged into dev-1.0.1 and removed dev/1.0.1-deprecated should be merged into dev-1.0.1 branch labels Jun 14, 2022

pingchunzhang added a commit to pingchunzhang/doris that referenced this pull request Jun 22, 2026

[test](microbench) Add some api of get_or_set (apache#9803)

e12afbf

Conversation

xinyiZzz commented May 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Problem Summary:

Upgrade:

Checklist(Required)

Further comments

Uh oh!

yangzhg May 27, 2022

Choose a reason for hiding this comment

Uh oh!

xinyiZzz May 27, 2022

Choose a reason for hiding this comment

Uh oh!

yangzhg Jun 3, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xinyiZzz commented Jun 2, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

morningman Jun 3, 2022

Choose a reason for hiding this comment

Uh oh!

xinyiZzz Jun 6, 2022

Choose a reason for hiding this comment

Uh oh!

xinyiZzz Jun 11, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

morningman left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jun 10, 2022

Uh oh!

github-actions Bot commented Jun 10, 2022

Uh oh!

yangzhg commented Jun 11, 2022

Uh oh!

xinyiZzz commented Jun 11, 2022

Uh oh!

yangzhg left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jun 13, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

xinyiZzz commented May 26, 2022 •

edited

Loading