You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Riza Suminto (Code Review)" <ge...@cloudera.org> on 2022/09/05 01:55:19 UTC
[Impala-ASF-CR] IMPALA-11548: Share same TTransport object for RPC input and output
Riza Suminto has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18938
Change subject: IMPALA-11548: Share same TTransport object for RPC input and output
......................................................................
IMPALA-11548: Share same TTransport object for RPC input and output
After the CPP thrift upgrade to version 0.16.0 by IMPALA-11384, we are
seeing long-running connections falling with the max message size error.
Additional tracing log in the thrift library and TAcceptQueueServer
reveals that TTransport's message size counter was continuously
decreasing over the lifetime of the connection and was never reset.
Further investigation point down to having separate input and output
TTransport object in TAcceptQueueServer::SetupConnection as the root
cause.
This patch fixes the issue by sharing the same TTransport object for
both Thrift RPC input and output in TAcceptQueueServer::SetupConnection.
Using the same TTransport causes the counter to be decremented and reset
on the same object. TSaslTransport had a caching algorithm in
TSaslServerTransport::Factory::getTransport() that guaranteed that the
underlying transport was shared between input and output. This is no
longer necessary and has been removed.
Testing:
- Pass core tests.
Change-Id: I199d6b0c6c62e940e131eb39d38a8a51b36e11c4
---
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/transport/TSaslServerTransport.cpp
M be/src/transport/TSaslServerTransport.h
3 files changed, 17 insertions(+), 42 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/18938/1
--
To view, visit http://gerrit.cloudera.org:8080/18938
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I199d6b0c6c62e940e131eb39d38a8a51b36e11c4
Gerrit-Change-Number: 18938
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-11548: Share same TTransport object for RPC input and output
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18938 )
Change subject: IMPALA-11548: Share same TTransport object for RPC input and output
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/11283/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/18938
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I199d6b0c6c62e940e131eb39d38a8a51b36e11c4
Gerrit-Change-Number: 18938
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Sep 2022 02:16:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11548: Share same TTransport object for RPC input and output
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/18938 )
Change subject: IMPALA-11548: Share same TTransport object for RPC input and output
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/18938
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I199d6b0c6c62e940e131eb39d38a8a51b36e11c4
Gerrit-Change-Number: 18938
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Sep 2022 12:56:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11548: Share same TTransport object for RPC input and output
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/18938 )
Change subject: IMPALA-11548: Share same TTransport object for RPC input and output
......................................................................
Patch Set 1:
(3 comments)
Thanks for the thorough investigation Riza and Joe! I only have the following very minor comments.
http://gerrit.cloudera.org:8080/#/c/18938/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18938/1//COMMIT_MSG@10
PS1, Line 10: falling with the max message size error
nit: Maybe it would be more accurate to change this to "closed by the Thrift server due to MaxMessageSize reached".
http://gerrit.cloudera.org:8080/#/c/18938/1//COMMIT_MSG@14
PS1, Line 14: point
nit: Change it to "points" or "pointed".
http://gerrit.cloudera.org:8080/#/c/18938/1/be/src/rpc/TAcceptQueueServer.cpp
File be/src/rpc/TAcceptQueueServer.cpp:
http://gerrit.cloudera.org:8080/#/c/18938/1/be/src/rpc/TAcceptQueueServer.cpp@242
PS1, Line 242: help with
nit: helps with simplifying
--
To view, visit http://gerrit.cloudera.org:8080/18938
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I199d6b0c6c62e940e131eb39d38a8a51b36e11c4
Gerrit-Change-Number: 18938
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Sep 2022 17:57:25 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11548: Share same TTransport object for RPC input and output
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/18938 )
Change subject: IMPALA-11548: Share same TTransport object for RPC input and output
......................................................................
Patch Set 3: Code-Review+2
This looks good to me! This is a nice cleanup.
--
To view, visit http://gerrit.cloudera.org:8080/18938
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I199d6b0c6c62e940e131eb39d38a8a51b36e11c4
Gerrit-Change-Number: 18938
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Sep 2022 23:49:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11548: Share same TTransport object for RPC input and output
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18938 )
Change subject: IMPALA-11548: Share same TTransport object for RPC input and output
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/18938
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I199d6b0c6c62e940e131eb39d38a8a51b36e11c4
Gerrit-Change-Number: 18938
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Sep 2022 04:46:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11548: Share same TTransport object for RPC input and output
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18938 )
Change subject: IMPALA-11548: Share same TTransport object for RPC input and output
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8536/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/18938
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I199d6b0c6c62e940e131eb39d38a8a51b36e11c4
Gerrit-Change-Number: 18938
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Sep 2022 23:58:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11548: Share same TTransport object for RPC input and output
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Hello Fang-Yu Rao, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18938
to look at the new patch set (#3).
Change subject: IMPALA-11548: Share same TTransport object for RPC input and output
......................................................................
IMPALA-11548: Share same TTransport object for RPC input and output
After the CPP thrift upgrade to version 0.16.0 by IMPALA-11384, we are
seeing long-running connections closed by the Thrift server due to
MaxMessageSize reached. Additional tracing log in the thrift library and
TAcceptQueueServer reveals that TTransport's message size counter was
continuously decreasing over the lifetime of the connection and was
never reset. Further investigation points down to having separate input
and output TTransport object in TAcceptQueueServer::SetupConnection as
the root cause.
This patch fixes the issue by sharing the same TTransport object for
both Thrift RPC input and output in TAcceptQueueServer::SetupConnection.
Using the same TTransport causes the counter to be decremented and reset
on the same object. TSaslTransport had a caching algorithm in
TSaslServerTransport::Factory::getTransport() that guaranteed that the
underlying transport was shared between input and output. This is no
longer necessary and has been removed.
Testing:
- Pass core tests.
Change-Id: I199d6b0c6c62e940e131eb39d38a8a51b36e11c4
---
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/TAcceptQueueServer.h
M be/src/transport/TSaslServerTransport.cpp
M be/src/transport/TSaslServerTransport.h
4 files changed, 27 insertions(+), 60 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/18938/3
--
To view, visit http://gerrit.cloudera.org:8080/18938
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I199d6b0c6c62e940e131eb39d38a8a51b36e11c4
Gerrit-Change-Number: 18938
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-11548: Share same TTransport object for RPC input and output
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18938 )
Change subject: IMPALA-11548: Share same TTransport object for RPC input and output
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/18938
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I199d6b0c6c62e940e131eb39d38a8a51b36e11c4
Gerrit-Change-Number: 18938
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Sep 2022 23:58:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11548: Share same TTransport object for RPC input and output
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/18938 )
Change subject: IMPALA-11548: Share same TTransport object for RPC input and output
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
It is good to see the old hack removed!
http://gerrit.cloudera.org:8080/#/c/18938/2/be/src/rpc/TAcceptQueueServer.cpp
File be/src/rpc/TAcceptQueueServer.cpp:
http://gerrit.cloudera.org:8080/#/c/18938/2/be/src/rpc/TAcceptQueueServer.cpp@246
PS2, Line 246: inputTransport = inputTransportFactory_->getTransport(client);
: outputTransport = inputTransport;
As the two are now the same, can you merge a variables? e.g. as transport or inputOutputTransport
The function CleanupAndClose could be also simplified to expect only a single transport. Currently we call close() two time on the same transport if there is some error, which may not be ok for some transports.
--
To view, visit http://gerrit.cloudera.org:8080/18938
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I199d6b0c6c62e940e131eb39d38a8a51b36e11c4
Gerrit-Change-Number: 18938
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Sep 2022 11:49:43 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11548: Share same TTransport object for RPC input and output
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18938 )
Change subject: IMPALA-11548: Share same TTransport object for RPC input and output
......................................................................
IMPALA-11548: Share same TTransport object for RPC input and output
After the CPP thrift upgrade to version 0.16.0 by IMPALA-11384, we are
seeing long-running connections closed by the Thrift server due to
MaxMessageSize reached. Additional tracing log in the thrift library and
TAcceptQueueServer reveals that TTransport's message size counter was
continuously decreasing over the lifetime of the connection and was
never reset. Further investigation points down to having separate input
and output TTransport object in TAcceptQueueServer::SetupConnection as
the root cause.
This patch fixes the issue by sharing the same TTransport object for
both Thrift RPC input and output in TAcceptQueueServer::SetupConnection.
Using the same TTransport causes the counter to be decremented and reset
on the same object. TSaslTransport had a caching algorithm in
TSaslServerTransport::Factory::getTransport() that guaranteed that the
underlying transport was shared between input and output. This is no
longer necessary and has been removed.
Testing:
- Pass core tests.
Change-Id: I199d6b0c6c62e940e131eb39d38a8a51b36e11c4
Reviewed-on: http://gerrit.cloudera.org:8080/18938
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/TAcceptQueueServer.h
M be/src/transport/TSaslServerTransport.cpp
M be/src/transport/TSaslServerTransport.h
4 files changed, 27 insertions(+), 60 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/18938
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I199d6b0c6c62e940e131eb39d38a8a51b36e11c4
Gerrit-Change-Number: 18938
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-11548: Share same TTransport object for RPC input and output
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Hello Fang-Yu Rao, Joe McDonnell, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18938
to look at the new patch set (#2).
Change subject: IMPALA-11548: Share same TTransport object for RPC input and output
......................................................................
IMPALA-11548: Share same TTransport object for RPC input and output
After the CPP thrift upgrade to version 0.16.0 by IMPALA-11384, we are
seeing long-running connections closed by the Thrift server due to
MaxMessageSize reached. Additional tracing log in the thrift library and
TAcceptQueueServer reveals that TTransport's message size counter was
continuously decreasing over the lifetime of the connection and was
never reset. Further investigation points down to having separate input
and output TTransport object in TAcceptQueueServer::SetupConnection as
the root cause.
This patch fixes the issue by sharing the same TTransport object for
both Thrift RPC input and output in TAcceptQueueServer::SetupConnection.
Using the same TTransport causes the counter to be decremented and reset
on the same object. TSaslTransport had a caching algorithm in
TSaslServerTransport::Factory::getTransport() that guaranteed that the
underlying transport was shared between input and output. This is no
longer necessary and has been removed.
Testing:
- Pass core tests.
Change-Id: I199d6b0c6c62e940e131eb39d38a8a51b36e11c4
---
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/transport/TSaslServerTransport.cpp
M be/src/transport/TSaslServerTransport.h
3 files changed, 17 insertions(+), 42 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/18938/2
--
To view, visit http://gerrit.cloudera.org:8080/18938
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I199d6b0c6c62e940e131eb39d38a8a51b36e11c4
Gerrit-Change-Number: 18938
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-11548: Share same TTransport object for RPC input and output
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18938 )
Change subject: IMPALA-11548: Share same TTransport object for RPC input and output
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/11297/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/18938
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I199d6b0c6c62e940e131eb39d38a8a51b36e11c4
Gerrit-Change-Number: 18938
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Sep 2022 10:37:46 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11548: Share same TTransport object for RPC input and output
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/18938 )
Change subject: IMPALA-11548: Share same TTransport object for RPC input and output
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/18938/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18938/1//COMMIT_MSG@10
PS1, Line 10: closed by the Thrift server due to
> nit: Maybe it would be more accurate to change this to "closed by the Thrif
Done
http://gerrit.cloudera.org:8080/#/c/18938/1//COMMIT_MSG@14
PS1, Line 14: nvest
> nit: Change it to "points" or "pointed".
Done
http://gerrit.cloudera.org:8080/#/c/18938/1/be/src/rpc/TAcceptQueueServer.cpp
File be/src/rpc/TAcceptQueueServer.cpp:
http://gerrit.cloudera.org:8080/#/c/18938/1/be/src/rpc/TAcceptQueueServer.cpp@242
PS1, Line 242: helps wit
> nit: helps with simplifying
Done
--
To view, visit http://gerrit.cloudera.org:8080/18938
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I199d6b0c6c62e940e131eb39d38a8a51b36e11c4
Gerrit-Change-Number: 18938
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Sep 2022 10:17:38 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11548: Share same TTransport object for RPC input and output
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/18938 )
Change subject: IMPALA-11548: Share same TTransport object for RPC input and output
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18938/2/be/src/rpc/TAcceptQueueServer.cpp
File be/src/rpc/TAcceptQueueServer.cpp:
http://gerrit.cloudera.org:8080/#/c/18938/2/be/src/rpc/TAcceptQueueServer.cpp@246
PS2, Line 246: outputProtocolFactory_->getProtocol(io_transport);
: shared_ptr<TProcessor> processor
> As the two are now the same, can you merge a variables? e.g. as transport o
Thank you for pointing this out! Renamed to io_transport.
--
To view, visit http://gerrit.cloudera.org:8080/18938
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I199d6b0c6c62e940e131eb39d38a8a51b36e11c4
Gerrit-Change-Number: 18938
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Sep 2022 12:15:55 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11548: Share same TTransport object for RPC input and output
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18938 )
Change subject: IMPALA-11548: Share same TTransport object for RPC input and output
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/11300/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/18938
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I199d6b0c6c62e940e131eb39d38a8a51b36e11c4
Gerrit-Change-Number: 18938
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Sep 2022 12:33:44 +0000
Gerrit-HasComments: No