You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Joe McDonnell (Code Review)" <ge...@cloudera.org> on 2018/02/13 22:45:48 UTC

[Impala-ASF-CR] KUDU-2296: Fix deserialization of messages larger than 64MB

Hello Kudu Jenkins, Todd Lipcon,

I'd like you to do a code review. Please visit

    http://gerrit.cloudera.org:8080/9313

to review the following change.


Change subject: KUDU-2296: Fix deserialization of messages larger than 64MB
......................................................................

KUDU-2296: Fix deserialization of messages larger than 64MB

Protobuf's CodedInputStream has a 64MB total byte limit by
default. When trying to deserialize messages larger than
this, ParseMessage() hits this limit and mistakenly
think that the packet is too short. This issue is dormant
due to Kudu's default rpc_max_message_size of 50MB.
However, Impala will be using a larger value for
rpc_max_message_size and requires this fix.

The fix is to override the default 64MB limit by calling
CodedInputStream::SetTotalByteLimit() with the buffer's
size.

Change-Id: I57d3f3ca6ec0aa8be0e67e6a13c4b560c9d2c63a
Reviewed-on: http://gerrit.cloudera.org:8080/9312
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M be/src/kudu/rpc/serialization.cc
1 file changed, 4 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/9313/1
-- 
To view, visit http://gerrit.cloudera.org:8080/9313
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I57d3f3ca6ec0aa8be0e67e6a13c4b560c9d2c63a
Gerrit-Change-Number: 9313
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[Impala-ASF-CR] KUDU-2296: Fix deserialization of messages larger than 64MB

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/9313 )

Change subject: KUDU-2296: Fix deserialization of messages larger than 64MB
......................................................................


Patch Set 1:

Clean cherry-pick


-- 
To view, visit http://gerrit.cloudera.org:8080/9313
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57d3f3ca6ec0aa8be0e67e6a13c4b560c9d2c63a
Gerrit-Change-Number: 9313
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 13 Feb 2018 22:46:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2296: Fix deserialization of messages larger than 64MB

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9313 )

Change subject: KUDU-2296: Fix deserialization of messages larger than 64MB
......................................................................


Patch Set 2: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/9313
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57d3f3ca6ec0aa8be0e67e6a13c4b560c9d2c63a
Gerrit-Change-Number: 9313
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 14 Feb 2018 03:01:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2296: Fix deserialization of messages larger than 64MB

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has removed Kudu Jenkins from this change.  ( http://gerrit.cloudera.org:8080/9313 )

Change subject: KUDU-2296: Fix deserialization of messages larger than 64MB
......................................................................


Removed reviewer Kudu Jenkins.
-- 
To view, visit http://gerrit.cloudera.org:8080/9313
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I57d3f3ca6ec0aa8be0e67e6a13c4b560c9d2c63a
Gerrit-Change-Number: 9313
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[Impala-ASF-CR] KUDU-2296: Fix deserialization of messages larger than 64MB

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9313 )

Change subject: KUDU-2296: Fix deserialization of messages larger than 64MB
......................................................................


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1932/


-- 
To view, visit http://gerrit.cloudera.org:8080/9313
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57d3f3ca6ec0aa8be0e67e6a13c4b560c9d2c63a
Gerrit-Change-Number: 9313
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 13 Feb 2018 23:18:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2296: Fix deserialization of messages larger than 64MB

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/9313 )

Change subject: KUDU-2296: Fix deserialization of messages larger than 64MB
......................................................................


Patch Set 2: Code-Review+2

Carry +2


-- 
To view, visit http://gerrit.cloudera.org:8080/9313
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57d3f3ca6ec0aa8be0e67e6a13c4b560c9d2c63a
Gerrit-Change-Number: 9313
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 13 Feb 2018 23:18:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2296: Fix deserialization of messages larger than 64MB

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/9313 )

Change subject: KUDU-2296: Fix deserialization of messages larger than 64MB
......................................................................


Patch Set 1: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/9313
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57d3f3ca6ec0aa8be0e67e6a13c4b560c9d2c63a
Gerrit-Change-Number: 9313
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 13 Feb 2018 22:51:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2296: Fix deserialization of messages larger than 64MB

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/9313 )

Change subject: KUDU-2296: Fix deserialization of messages larger than 64MB
......................................................................

KUDU-2296: Fix deserialization of messages larger than 64MB

Protobuf's CodedInputStream has a 64MB total byte limit by
default. When trying to deserialize messages larger than
this, ParseMessage() hits this limit and mistakenly
think that the packet is too short. This issue is dormant
due to Kudu's default rpc_max_message_size of 50MB.
However, Impala will be using a larger value for
rpc_max_message_size and requires this fix.

The fix is to override the default 64MB limit by calling
CodedInputStream::SetTotalByteLimit() with the buffer's
size.

Change-Id: I57d3f3ca6ec0aa8be0e67e6a13c4b560c9d2c63a
Reviewed-on: http://gerrit.cloudera.org:8080/9312
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
Reviewed-on: http://gerrit.cloudera.org:8080/9313
Reviewed-by: Joe McDonnell <jo...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/kudu/rpc/serialization.cc
1 file changed, 4 insertions(+), 0 deletions(-)

Approvals:
  Joe McDonnell: Looks good to me, approved
  Impala Public Jenkins: Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/9313
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I57d3f3ca6ec0aa8be0e67e6a13c4b560c9d2c63a
Gerrit-Change-Number: 9313
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>