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 2024/04/28 19:45:56 UTC
[Impala-ASF-CR] PROTOTYPE: IMPALA-13020: Change thrift rpc max message size to int64 t
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21367
Change subject: PROTOTYPE: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
PROTOTYPE: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
---
M be/src/common/init.cc
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-util.cc
M be/src/rpc/thrift-util.h
M be/src/util/backend-gflag-util.cc
M bin/impala-config.sh
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
9 files changed, 19 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/21367/1
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 02 May 2024 01:59:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13020 (part 1): Change thrift rpc max message size to int64 t
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: IMPALA-13020 (part 1): Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 5: Verified+1 Code-Review+2
I'm going to go ahead with this. The top of the stack passed precommit.
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 17 May 2024 19:07:45 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] PROTOTYPE: IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: PROTOTYPE: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 1:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/16047/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sun, 28 Apr 2024 19:52:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21367/4/be/src/rpc/thrift-util.cc
File be/src/rpc/thrift-util.cc:
http://gerrit.cloudera.org:8080/#/c/21367/4/be/src/rpc/thrift-util.cc@61
PS4, Line 61: DEFINE_int64(thrift_rpc_max_message_size, 4L * 1024 * 1024 * 1024,
Yeah, it'd be perfect if we can split up them. We already have a pretty large limit. A query sent from the client shouldn't need that large.
> Is 8GB reasonable?
It's hard to estimate the ratio since it depends on many factors, e.g. number of partitions. However, it's reported in IMPALA-12350 that 800MB could grow to 3.4GB after upgrade. So 4x larger is possible in reality and 4GB x 4 = 16GB might be safer.
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 07 May 2024 04:02:59 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13020 (part 1): Change thrift rpc max message size to int64 t
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: IMPALA-13020 (part 1): Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/16170/ : 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/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 10 May 2024 23:28:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/16078/ : 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/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 01 May 2024 20:30:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] PROTOTYPE: IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: PROTOTYPE: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10588/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sun, 28 Apr 2024 20:46:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13020 (part 1): Change thrift rpc max message size to int64 t
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21367
to look at the new patch set (#5).
Change subject: IMPALA-13020 (part 1): Change thrift_rpc_max_message_size to int64_t
......................................................................
IMPALA-13020 (part 1): Change thrift_rpc_max_message_size to int64_t
Thrift 0.16.0 introduced a max message size to protect
receivers against a malicious message allocating large
amounts of memory. That limit is a 32-bit signed integer,
so the max value is 2GB. Impala introduced the
thrift_rpc_max_message_size startup option to set that
for Impala's thrift servers.
There are times when Impala wants to send a message that
is larger than 2GB. In particular, the catalog-update
topic for the statestore can exceed 2GBs when there is
a lot of metadata loaded using the old v1 catalog. When
there is a 2GB max message size, the statestore can create
and send a >2GB message, but the impalads will reject
it. This can lead to impalads having stale metadata.
This switches to a patched Thrift that uses an int64_t
for the max message size for C++ code. It does not modify
the limit.
The MaxMessageSize error was being swallowed in TAcceptQueueServer.cpp,
so this fixes that location to always print MaxMessageSize
exceptions.
This is only patching the Thrift C++ library. It does not
patch the Thrift Java library. There are a few reasons for
that:
- This specific issue involves C++ to C++ communication and
will be solved by patching the C++ library.
- C++ is easy to patch as it is built via the native-toolchain.
There is no corresponding mechanism for patching our Java
dependencies (though one could be developed).
- Java modifications have implications for other dependencies
like Hive which use Thrift to communicate with HMS.
For the Java code that uses max message size, this converts
the 64-bit value to 32-bit value by capping the value at
Integer.MAX_VALUE.
Testing:
- Added enough tables to produce a >2GB catalog-topic and
restarted an impalad with a higher limit specific. Without
the patch, the catalog-topic update would be rejected by the
impalad. With the patch, it succeeds.
Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
---
M be/src/common/init.cc
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-util.cc
M be/src/rpc/thrift-util.h
M be/src/util/backend-gflag-util.cc
M bin/impala-config.sh
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
9 files changed, 28 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/21367/5
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-13020 (part 1): Change thrift rpc max message size to int64 t
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: IMPALA-13020 (part 1): Change thrift_rpc_max_message_size to int64_t
......................................................................
IMPALA-13020 (part 1): Change thrift_rpc_max_message_size to int64_t
Thrift 0.16.0 introduced a max message size to protect
receivers against a malicious message allocating large
amounts of memory. That limit is a 32-bit signed integer,
so the max value is 2GB. Impala introduced the
thrift_rpc_max_message_size startup option to set that
for Impala's thrift servers.
There are times when Impala wants to send a message that
is larger than 2GB. In particular, the catalog-update
topic for the statestore can exceed 2GBs when there is
a lot of metadata loaded using the old v1 catalog. When
there is a 2GB max message size, the statestore can create
and send a >2GB message, but the impalads will reject
it. This can lead to impalads having stale metadata.
This switches to a patched Thrift that uses an int64_t
for the max message size for C++ code. It does not modify
the limit.
The MaxMessageSize error was being swallowed in TAcceptQueueServer.cpp,
so this fixes that location to always print MaxMessageSize
exceptions.
This is only patching the Thrift C++ library. It does not
patch the Thrift Java library. There are a few reasons for
that:
- This specific issue involves C++ to C++ communication and
will be solved by patching the C++ library.
- C++ is easy to patch as it is built via the native-toolchain.
There is no corresponding mechanism for patching our Java
dependencies (though one could be developed).
- Java modifications have implications for other dependencies
like Hive which use Thrift to communicate with HMS.
For the Java code that uses max message size, this converts
the 64-bit value to 32-bit value by capping the value at
Integer.MAX_VALUE.
Testing:
- Added enough tables to produce a >2GB catalog-topic and
restarted an impalad with a higher limit specific. Without
the patch, the catalog-topic update would be rejected by the
impalad. With the patch, it succeeds.
Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Reviewed-on: http://gerrit.cloudera.org:8080/21367
Reviewed-by: Michael Smith <mi...@cloudera.com>
Reviewed-by: Joe McDonnell <jo...@cloudera.com>
Tested-by: Joe McDonnell <jo...@cloudera.com>
---
M be/src/common/init.cc
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-util.cc
M be/src/rpc/thrift-util.h
M be/src/util/backend-gflag-util.cc
M bin/impala-config.sh
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
9 files changed, 28 insertions(+), 18 deletions(-)
Approvals:
Michael Smith: Looks good to me, but someone else must approve
Joe McDonnell: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21367
to look at the new patch set (#3).
Change subject: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
Thrift 0.16.0 introduced a max message size to protect
receivers against a malicious message allocating large
amounts of memory. That limit is a 32-bit signed integer,
so the max value is 2GB. Impala introduced the
thrift_rpc_max_message_size startup option to set that
for Impala's thrift servers.
There are times when Impala wants to send a message that
is larger than 2GB. In particular, the catalog-update
topic for the statestore can exceed 2GBs when there is
a lot of metadata loaded using the old v1 catalog. When
there is a 2GB max message size, the statestore can create
and send a >2GB message, but the impalads will reject
it. This can lead to impalads having stale metadata.
This switches to a patched Thrift that uses an int64_t
for the max message size for C++ code. It also modifies
thrift_rpc_max_message_size to be an int64_t and increases
the default max message size to 4GB.
The MaxMessageSize error was being swallowed in TAcceptQueueServer.cpp,
so this fixes that location to always print MaxMessageSize
exceptions.
This is only patching the Thrift C++ library. It does not
patch the Thrift Java library. There are a few reasons for
that:
- This specific issue involves C++ to C++ communication and
will be solved by patching the C++ library.
- C++ is easy to patch as it is built via the native-toolchain.
There is no corresponding mechanism for patching our Java
dependencies (though one could be developed).
- Java modifications have implications for other dependencies
like Hive which use Thrift to communicate with HMS.
For the Java code that uses max message size, this converts
the 64-bit value to 32-bit value by capping the value at
Integer.MAX_VALUE.
Testing:
- Added enough tables to produce a >2GB catalog-topic and
restarted an impalad. Without the patch, the catalog-topic
update would be rejected by the impalad. With the patch,
it succeeds.
Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
---
M be/src/common/init.cc
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-util.cc
M be/src/rpc/thrift-util.h
M be/src/util/backend-gflag-util.cc
M bin/impala-config.sh
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
9 files changed, 25 insertions(+), 20 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/21367/3
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
[Impala-ASF-CR] IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21367/4/be/src/rpc/thrift-util.cc
File be/src/rpc/thrift-util.cc:
http://gerrit.cloudera.org:8080/#/c/21367/4/be/src/rpc/thrift-util.cc@61
PS4, Line 61: DEFINE_int64(thrift_rpc_max_message_size, 4L * 1024 * 1024 * 1024,
> Can we set it to unlimited or a larger value? We have customers using impal
Is 8GB reasonable?
Setting it to unlimited means there is no protection from the CVE that prompted it. That is probably ok for messages that we send within the Impala cluster, but we use the same limit for external clients.
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 06 May 2024 18:03:58 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21367/4/be/src/rpc/thrift-util.cc
File be/src/rpc/thrift-util.cc:
http://gerrit.cloudera.org:8080/#/c/21367/4/be/src/rpc/thrift-util.cc@61
PS4, Line 61: DEFINE_int64(thrift_rpc_max_message_size, 4L * 1024 * 1024 * 1024,
> Is 8GB reasonable?
Could we split up those limits?
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 06 May 2024 18:42:42 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21367/3/be/src/rpc/TAcceptQueueServer.cpp
File be/src/rpc/TAcceptQueueServer.cpp:
http://gerrit.cloudera.org:8080/#/c/21367/3/be/src/rpc/TAcceptQueueServer.cpp@94
PS3, Line 94: std::string(ttx.what()).find("MaxMessageSize") != std::string::npos) {
Could we give a recommendation to increase thrift_rpc_max_message_size?
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 01 May 2024 20:16:52 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/16079/ : 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/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 01 May 2024 21:03:00 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 01 May 2024 20:49:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10597/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 01 May 2024 20:57:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] PROTOTYPE: IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: PROTOTYPE: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21367/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:
http://gerrit.cloudera.org:8080/#/c/21367/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java@411
PS2, Line 411: return (int) Math.min(backendCfg_.thrift_rpc_max_message_size, Integer.MAX_VALUE);
> No major reason. Mainly, I'm doing C++ because C++ is easy for us to patch
That'd be useful to clarify in the commit message.
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Mon, 29 Apr 2024 17:35:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] PROTOTYPE: IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: PROTOTYPE: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21367/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:
http://gerrit.cloudera.org:8080/#/c/21367/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java@411
PS2, Line 411: return (int) Math.min(backendCfg_.thrift_rpc_max_message_size, Integer.MAX_VALUE);
What's the rationale for not increasing it?
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Mon, 29 Apr 2024 16:47:43 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] PROTOTYPE: IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: PROTOTYPE: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21367/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:
http://gerrit.cloudera.org:8080/#/c/21367/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java@411
PS2, Line 411: return (int) Math.min(backendCfg_.thrift_rpc_max_message_size, Integer.MAX_VALUE);
> What's the rationale for not increasing it?
No major reason. Mainly, I'm doing C++ because C++ is easy for us to patch and the specific bug for IMPALA-13020 involves C++ to C++ communication.
We should think about modifying the Java library to support a higher limit. We haven't established a way to patch the Java library yet. We also have to think about how Hive uses Thrift and whether it would need to be recompiled if we change the Thrift signature.
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Mon, 29 Apr 2024 17:16:11 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13020 (part 1): Change thrift rpc max message size to int64 t
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: IMPALA-13020 (part 1): Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/16175/ : 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/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 17 May 2024 18:56:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] PROTOTYPE: IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: PROTOTYPE: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/16048/ : 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/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sun, 28 Apr 2024 20:21:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21367/4/be/src/rpc/thrift-util.cc
File be/src/rpc/thrift-util.cc:
http://gerrit.cloudera.org:8080/#/c/21367/4/be/src/rpc/thrift-util.cc@61
PS4, Line 61: DEFINE_int64(thrift_rpc_max_message_size, 4L * 1024 * 1024 * 1024,
Can we set it to unlimited or a larger value? We have customers using impala-3.4 and their catalog-topic size reaches 4GB. Once they upgrade to Impala-4.x, the catalog-topic size will grow larger since impala-3.4 has better compression rate on catalog objects. The reason is in Impala-3.4, partition objects are sent inside the table object so more duplicated bytes can be saved. In Impala-4.x, partition objects are compressed and sent individually for incremental updates (IMPALA-3127).
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 06 May 2024 06:06:10 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13020 (part 1): Change thrift rpc max message size to int64 t
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: IMPALA-13020 (part 1): Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21367/5/be/src/rpc/thrift-util.cc
File be/src/rpc/thrift-util.cc:
http://gerrit.cloudera.org:8080/#/c/21367/5/be/src/rpc/thrift-util.cc@63
PS5, Line 63: "Default to the upper limit of 2147483647 bytes (~2GB). "
"Default to the upper limit" seems misleading, as it's no longer the upper limit for this option.
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 14 May 2024 17:29:40 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13020 (part 1): Change thrift rpc max message size to int64 t
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: IMPALA-13020 (part 1): Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 5: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21367/5/be/src/rpc/thrift-util.cc
File be/src/rpc/thrift-util.cc:
http://gerrit.cloudera.org:8080/#/c/21367/5/be/src/rpc/thrift-util.cc@63
PS5, Line 63: "Default to the upper limit of 2147483647 bytes (~2GB). "
> "Default to the upper limit" seems misleading, as it's no longer the upper
This is all reworded in the next patch, so it's probably fine.
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 17 May 2024 17:49:14 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] PROTOTYPE: IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21367
to look at the new patch set (#2).
Change subject: PROTOTYPE: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
PROTOTYPE: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
---
M be/src/common/init.cc
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-util.cc
M be/src/rpc/thrift-util.h
M be/src/util/backend-gflag-util.cc
M bin/impala-config.sh
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
9 files changed, 20 insertions(+), 19 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/21367/2
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] PROTOTYPE: IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: PROTOTYPE: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 29 Apr 2024 01:48:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21367
to look at the new patch set (#4).
Change subject: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
Thrift 0.16.0 introduced a max message size to protect
receivers against a malicious message allocating large
amounts of memory. That limit is a 32-bit signed integer,
so the max value is 2GB. Impala introduced the
thrift_rpc_max_message_size startup option to set that
for Impala's thrift servers.
There are times when Impala wants to send a message that
is larger than 2GB. In particular, the catalog-update
topic for the statestore can exceed 2GBs when there is
a lot of metadata loaded using the old v1 catalog. When
there is a 2GB max message size, the statestore can create
and send a >2GB message, but the impalads will reject
it. This can lead to impalads having stale metadata.
This switches to a patched Thrift that uses an int64_t
for the max message size for C++ code. It also modifies
thrift_rpc_max_message_size to be an int64_t and increases
the default max message size to 4GB.
The MaxMessageSize error was being swallowed in TAcceptQueueServer.cpp,
so this fixes that location to always print MaxMessageSize
exceptions.
This is only patching the Thrift C++ library. It does not
patch the Thrift Java library. There are a few reasons for
that:
- This specific issue involves C++ to C++ communication and
will be solved by patching the C++ library.
- C++ is easy to patch as it is built via the native-toolchain.
There is no corresponding mechanism for patching our Java
dependencies (though one could be developed).
- Java modifications have implications for other dependencies
like Hive which use Thrift to communicate with HMS.
For the Java code that uses max message size, this converts
the 64-bit value to 32-bit value by capping the value at
Integer.MAX_VALUE.
Testing:
- Added enough tables to produce a >2GB catalog-topic and
restarted an impalad. Without the patch, the catalog-topic
update would be rejected by the impalad. With the patch,
it succeeds.
Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
---
M be/src/common/init.cc
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-util.cc
M be/src/rpc/thrift-util.h
M be/src/util/backend-gflag-util.cc
M bin/impala-config.sh
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
9 files changed, 30 insertions(+), 20 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/21367/4
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-13020: Change thrift rpc max message size to int64 t
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 )
Change subject: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21367/3/be/src/rpc/TAcceptQueueServer.cpp
File be/src/rpc/TAcceptQueueServer.cpp:
http://gerrit.cloudera.org:8080/#/c/21367/3/be/src/rpc/TAcceptQueueServer.cpp@94
PS3, Line 94: std::string(ttx.what()).find("MaxMessageSize") != std::string::npos) {
> Could we give a recommendation to increase thrift_rpc_max_message_size?
Done
--
To view, visit http://gerrit.cloudera.org:8080/21367
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87
Gerrit-Change-Number: 21367
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 01 May 2024 20:38:54 +0000
Gerrit-HasComments: Yes