[Impala-ASF-CR] IMPALA-7968, Part 1: JSON serialization framework
Paul Rogers has posted comments on this change. (
http://gerrit.cloudera.org:8080/12079 )
Change subject: IMPALA-7968, Part 1: JSON serialization framework
..
Patch Set 5:
(5 comments)
Thanks for the reviews; addressed comments.
After testing, turns out we can get the Jackson serializer to do pretty much
what is wanted, so have removed the bespoke serializer to keep things simple.
http://gerrit.cloudera.org:8080/#/c/12079/3/fe/src/main/java/org/apache/impala/common/serialize/ObjectSerializer.java
File fe/src/main/java/org/apache/impala/common/serialize/ObjectSerializer.java:
http://gerrit.cloudera.org:8080/#/c/12079/3/fe/src/main/java/org/apache/impala/common/serialize/ObjectSerializer.java@83
PS3, Line 83: objectList
> shouldn't this be stringList?
Done
http://gerrit.cloudera.org:8080/#/c/12079/3/fe/src/main/java/org/apache/impala/common/serialize/ObjectSerializer.java@88
PS3, Line 88: objectList
> should this be scalarList?
Done
http://gerrit.cloudera.org:8080/#/c/12079/3/fe/src/main/java/org/apache/impala/common/serialize/ToJsonOptions.java
File fe/src/main/java/org/apache/impala/common/serialize/ToJsonOptions.java:
http://gerrit.cloudera.org:8080/#/c/12079/3/fe/src/main/java/org/apache/impala/common/serialize/ToJsonOptions.java@34
PS3, Line 34: public ToJsonOptions showSource(boolean flag)
: { showSource_ = flag; return this; }
> Sorry if I wasn't clear. I meant to use the standard style.
Done
http://gerrit.cloudera.org:8080/#/c/12079/2/fe/src/test/java/org/apache/impala/common/serialize/TreeFormatterTest.java
File fe/src/test/java/org/apache/impala/common/serialize/TreeFormatterTest.java:
http://gerrit.cloudera.org:8080/#/c/12079/2/fe/src/test/java/org/apache/impala/common/serialize/TreeFormatterTest.java@32
PS2, Line 32:
> nit: remove new line
This file is now removed, but made the same fix in the new Jackson-based
version.
http://gerrit.cloudera.org:8080/#/c/12079/2/fe/src/test/java/org/apache/impala/common/serialize/TreeFormatterTest.java@39
PS2, Line 39:
:
:
:
:
:
:
> What do you think about this?
Handy trick I'll use in the future. For this one, the class approach allowed a
very simple port from the original bespoke implementation to the Jackson
implementation which Phil suggested. I'd have to do quite a bit of rewrite to
adopt the above version. The above version has more redundancy than the current
revised version.
Please take a look. Happy to do the rewrite if necessary, but let's make that
decision based on the current code.
--
To view, visit http://gerrit.cloudera.org:8080/12079
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3101c2708bf6cf4bec61af83a5db9b6d70ddd9c
Gerrit-Change-Number: 12079
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Comment-Date: Thu, 20 Dec 2018 02:26:18 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] Added timeout to run-all-tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12086 ) Change subject: Added timeout to run-all-tests .. Patch Set 2: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/1650/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/12086 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iefd3e16a973709f27aeffc7a15bcab8fcdbb349b Gerrit-Change-Number: 12086 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 20 Dec 2018 01:33:36 + Gerrit-HasComments: No
[Impala-ASF-CR] Added timeout to run-all-tests
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/12086 ) Change subject: Added timeout to run-all-tests .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12086 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iefd3e16a973709f27aeffc7a15bcab8fcdbb349b Gerrit-Change-Number: 12086 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 20 Dec 2018 00:50:47 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12108 ) Change subject: IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 Gerrit-Change-Number: 12108 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 20 Dec 2018 00:48:54 + Gerrit-HasComments: No
[Impala-ASF-CR] Added timeout to run-all-tests
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/12086 ) Change subject: Added timeout to run-all-tests .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/12086 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iefd3e16a973709f27aeffc7a15bcab8fcdbb349b Gerrit-Change-Number: 12086 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 20 Dec 2018 00:48:00 + Gerrit-HasComments: No
[Impala-ASF-CR] Added timeout to run-all-tests
Hello Lars Volker, Michael Brown, Philip Zeyliger, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12086 to look at the new patch set (#2). Change subject: Added timeout to run-all-tests .. Added timeout to run-all-tests Change-Id: Iefd3e16a973709f27aeffc7a15bcab8fcdbb349b --- M bin/impala-config.sh A bin/run-all-tests-timeout-check.sh M bin/run-all-tests.sh 3 files changed, 67 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/12086/2 -- To view, visit http://gerrit.cloudera.org:8080/12086 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iefd3e16a973709f27aeffc7a15bcab8fcdbb349b Gerrit-Change-Number: 12086 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] Added timeout to run-all-tests
Bikramjeet Vig has posted comments on this change. (
http://gerrit.cloudera.org:8080/12086 )
Change subject: Added timeout to run-all-tests
..
Patch Set 1:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests-timeout-check.sh
File bin/run-all-tests-timeout-check.sh:
http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests-timeout-check.sh@8
PS1, Line 8: echo "Expected timeout value as an argument"
> add exit 1?
Done
http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests-timeout-check.sh@22
PS1, Line 22: SECONDS=0
> This confused me!
Done
http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests-timeout-check.sh@24
PS1, Line 24: sleep 60
> I think you can reasonably just sleep 1 here and avoid the complication of
Done
http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests-timeout-check.sh@25
PS1, Line 25: [[ -z "$(ps -p $PPID -o pid=)" ]] && exit
> This is just:
Done
http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests-timeout-check.sh@45
PS1, Line 45: gdb -ex "set pagination 0" -ex "thread apply all bt" --batch
-p $pid > \
> I'm surprised we need "set pagination" in addition to --batch.
Done
http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests-timeout-check.sh@52
PS1, Line 52: "Test run timed out"
> Explain where to find information? ($IMPALA_TIMEOUT_LOGS_DIR)
Done
http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests.sh
File bin/run-all-tests.sh:
http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests.sh@34
PS1, Line 34: : ${TIMEOUT_FOR_RUN_ALL_TESTS_MINS:=0}
> Any reason not to default this to something high, like 20 hours?
no particular reason, just did not want to modify the current behavior. will
change the default to 20hrs as you suggested.
--
To view, visit http://gerrit.cloudera.org:8080/12086
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iefd3e16a973709f27aeffc7a15bcab8fcdbb349b
Gerrit-Change-Number: 12086
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Michael Brown
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Comment-Date: Thu, 20 Dec 2018 00:43:02 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7213: Use separate network plane for DataStream and Control services
Michael Ho has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12107 ) Change subject: IMPALA-7213: Use separate network plane for DataStream and Control services .. IMPALA-7213: Use separate network plane for DataStream and Control services This change is a follow up for a review comment in https://gerrit.cloudera.org/#/c/10855/ about separating the TCP connections of DataStream and Control services so that control commands don't get blocked behind large payloads being sent in the DataStream services. Testing done: exhaustive build Change-Id: I774f4a0e2cfedc4dba72cde4f5d28898cdbdc236 Reviewed-on: http://gerrit.cloudera.org:8080/12107 Reviewed-by: Michael Ho Tested-by: Impala Public Jenkins --- M be/src/service/control-service.cc M be/src/service/data-stream-service.cc 2 files changed, 8 insertions(+), 2 deletions(-) Approvals: Michael Ho: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/12107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I774f4a0e2cfedc4dba72cde4f5d28898cdbdc236 Gerrit-Change-Number: 12107 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] Add "network plane" as part of ConnectionId
Michael Ho has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12106 ) Change subject: Add "network_plane" as part of ConnectionId .. Add "network_plane" as part of ConnectionId The motivation for doing so is to allow N services on the same host to be multiplexed on M different connections. For instance, a server may host multiple KRPC based services: one for control command and one for data transfer. Separating the connections between the control channel and the data channel prevents unnecessary delays of the control commands due to being stuck behind large data transfers from client to server. By default, the network_plane of a new ConnectionId is not set. A user can change it to a different value by calling Proxy::set_network_plane() on the ConnectionId. Change-Id: I6767e631fd9530ea54f5ed63ff4c8c179ab216b2 Reviewed-on: http://gerrit.cloudera.org:8080/11681 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins Reviewed-on: http://gerrit.cloudera.org:8080/12106 Reviewed-by: Thomas Marshall Tested-by: Michael Ho --- M be/src/kudu/rpc/connection_id.cc M be/src/kudu/rpc/connection_id.h M be/src/kudu/rpc/messenger.h M be/src/kudu/rpc/proxy.cc M be/src/kudu/rpc/proxy.h M be/src/kudu/rpc/reactor.h M be/src/kudu/rpc/rpc-test.cc 7 files changed, 138 insertions(+), 31 deletions(-) Approvals: Thomas Marshall: Looks good to me, approved Michael Ho: Verified -- To view, visit http://gerrit.cloudera.org:8080/12106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I6767e631fd9530ea54f5ed63ff4c8c179ab216b2 Gerrit-Change-Number: 12106 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] Add "network plane" as part of ConnectionId
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12106 ) Change subject: Add "network_plane" as part of ConnectionId .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6767e631fd9530ea54f5ed63ff4c8c179ab216b2 Gerrit-Change-Number: 12106 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Thu, 20 Dec 2018 00:27:24 + Gerrit-HasComments: No
[Impala-ASF-CR] Add "network plane" as part of ConnectionId
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12106 ) Change subject: Add "network_plane" as part of ConnectionId .. Patch Set 1: Verified by https://jenkins.impala.io/job/gerrit-verify-dryrun/3588/ -- To view, visit http://gerrit.cloudera.org:8080/12106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6767e631fd9530ea54f5ed63ff4c8c179ab216b2 Gerrit-Change-Number: 12106 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Thu, 20 Dec 2018 00:27:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7213: Use separate network plane for DataStream and Control services
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12107 ) Change subject: IMPALA-7213: Use separate network plane for DataStream and Control services .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I774f4a0e2cfedc4dba72cde4f5d28898cdbdc236 Gerrit-Change-Number: 12107 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 19 Dec 2018 23:37:19 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags
Fredy Wijaya has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/12108 ) Change subject: IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags .. IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags This patch adds an option in gflags::GetAllFlags() to show all flags including hidden flags only if the hidden flags were modified from their default values. Testing: ./build.sh gflags 2.2.0-p2 Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 --- A source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch 1 file changed, 112 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/08/12108/3 -- To view, visit http://gerrit.cloudera.org:8080/12108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 Gerrit-Change-Number: 12108 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7265: Add parameter to cache remote HDFS file handles
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/12111 ) Change subject: IMPALA-7265: Add parameter to cache remote HDFS file handles .. Patch Set 1: (1 comment) Thanks! Do we want to have a custom cluster test that exercises this code path? http://gerrit.cloudera.org:8080/#/c/12111/1/be/src/runtime/io/scan-range.cc File be/src/runtime/io/scan-range.cc: http://gerrit.cloudera.org:8080/#/c/12111/1/be/src/runtime/io/scan-range.cc@202 PS1, Line 202: // 2. The file must not be on S3, ADLS, or ABFS. Perhaps isntead of checking that it's not S3, ADLS, or ABFS, we check explicitly that it's either Local Hdfs or that it's remote HDFS (if the given option is enabled). I think this translates to: is_file_handle_caching_enabled() && (expected_local_ || (FLAGS_cache_remote_file_handles && disk_id_ == io_mgr->RemoteDfsDiskId()) -- To view, visit http://gerrit.cloudera.org:8080/12111 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I549f007432a01ca52fa8093d458a220bba02e1d9 Gerrit-Change-Number: 12111 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 19 Dec 2018 22:52:51 + Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12108 ) Change subject: IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/12108/2/source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch File source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch: http://gerrit.cloudera.org:8080/#/c/12108/2/source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch@41 PS2, Line 41: + if (i->second->Hidden() && !i->second->Modified()) continue; > I think you can simplify this to I don't think it's the same. When the flag is hidden and show_hidden is true, the condition above becomes false. -- To view, visit http://gerrit.cloudera.org:8080/12108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 Gerrit-Change-Number: 12108 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Dec 2018 22:45:27 + Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12108 ) Change subject: IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/12108/2/source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch File source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch: http://gerrit.cloudera.org:8080/#/c/12108/2/source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch@41 PS2, Line 41: + if (i->second->Hidden() && !i->second->Modified()) continue; I think you can simplify this to if (i->second->Hidden() && !show_hidden && !i->second->Modified()) continue; -- To view, visit http://gerrit.cloudera.org:8080/12108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 Gerrit-Change-Number: 12108 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Dec 2018 22:30:10 + Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags
Fredy Wijaya has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/12108 ) Change subject: IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags .. IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags This patch adds an option in gflags::GetAllFlags() to show all flags including hidden flags only if the hidden flags were modified from their default values. Testing: ./build.sh gflags 2.2.0-p2 Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 --- A source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch 1 file changed, 114 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/08/12108/2 -- To view, visit http://gerrit.cloudera.org:8080/12108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 Gerrit-Change-Number: 12108 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7994: Prevent test insert large string from causing OOM issues
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12110 ) Change subject: IMPALA-7994: Prevent test_insert_large_string from causing OOM issues .. IMPALA-7994: Prevent test_insert_large_string from causing OOM issues test_insert_large_string uses upto 4GB of untracked memory which results in random OOMs during exhaustive testing on release builds. Queries run faster on release builds which might result in a different set of tests running together when compared to those on debug builds. This can result in queries requiring more memory running together with test_insert_large_string and eventually encounter OOM errors. Testing: Successfully ran exhaustive tests twice on release build. Change-Id: I6c950f6860b2f86865dbc5ce60055175e2c0bebc Reviewed-on: http://gerrit.cloudera.org:8080/12110 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M tests/query_test/test_insert.py 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/12110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I6c950f6860b2f86865dbc5ce60055175e2c0bebc Gerrit-Change-Number: 12110 Gerrit-PatchSet: 3 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7994: Prevent test insert large string from causing OOM issues
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12110 ) Change subject: IMPALA-7994: Prevent test_insert_large_string from causing OOM issues .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c950f6860b2f86865dbc5ce60055175e2c0bebc Gerrit-Change-Number: 12110 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 19 Dec 2018 21:28:04 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags
Lars Volker has posted comments on this change. (
http://gerrit.cloudera.org:8080/12108 )
Change subject: IMPALA-6521: Patch gflags to have an option to show all flags
including hidden flags
..
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12108/1/source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch
File
source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch:
http://gerrit.cloudera.org:8080/#/c/12108/1/source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch@63
PS1, Line 63: +TEST(GetAllHiddenFlagsTest, BaseTest) {
> test_bool_hidden is set by this: https://github.com/cloudera/native-toolcha
Thanks
--
To view, visit http://gerrit.cloudera.org:8080/12108
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6
Gerrit-Change-Number: 12108
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 19 Dec 2018 20:55:31 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12004 ) Change subject: IMPALA-7889: Write new logical types in Parquet .. Patch Set 11: Code-Review+1 (2 comments) Looks like a test failed, too. http://gerrit.cloudera.org:8080/#/c/12004/11/be/src/exec/parquet/parquet-metadata-utils.cc File be/src/exec/parquet/parquet-metadata-utils.cc: http://gerrit.cloudera.org:8080/#/c/12004/11/be/src/exec/parquet/parquet-metadata-utils.cc@323 PS11, Line 323: case TYPE_DECIMAL: nit: indent case statements by 2 spaces (see rest of file). http://gerrit.cloudera.org:8080/#/c/12004/11/be/src/exec/parquet/parquet-metadata-utils.cc@354 PS11, Line 354: // Leave logical and converted types empty for other types. Have you considered enumerating the other types here? Then you can DCHECK on an unknown type. That way we make future mistakes less likely by forgetting to update this statement. -- To view, visit http://gerrit.cloudera.org:8080/12004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 Gerrit-Change-Number: 12004 Gerrit-PatchSet: 11 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Anonymous Coward (359) Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Zoltan Ivanfi Gerrit-Comment-Date: Wed, 19 Dec 2018 20:51:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12004 ) Change subject: IMPALA-7889: Write new logical types in Parquet .. Patch Set 11: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3586/ -- To view, visit http://gerrit.cloudera.org:8080/12004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 Gerrit-Change-Number: 12004 Gerrit-PatchSet: 11 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Anonymous Coward (359) Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Zoltan Ivanfi Gerrit-Comment-Date: Wed, 19 Dec 2018 20:37:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Lars Volker has removed a vote on this change. Change subject: IMPALA-7889: Write new logical types in Parquet .. Removed Code-Review+2 by Impala Public Jenkins -- To view, visit http://gerrit.cloudera.org:8080/12004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 Gerrit-Change-Number: 12004 Gerrit-PatchSet: 11 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Anonymous Coward (359) Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Zoltan Ivanfi
[Impala-ASF-CR] IMPALA-7265: Add parameter to cache remote HDFS file handles
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12111 ) Change subject: IMPALA-7265: Add parameter to cache remote HDFS file handles .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1648/ : 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/12111 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I549f007432a01ca52fa8093d458a220bba02e1d9 Gerrit-Change-Number: 12111 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 19 Dec 2018 20:06:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7213: Use separate network plane for DataStream and Control services
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12107 ) Change subject: IMPALA-7213: Use separate network plane for DataStream and Control services .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1649/ : 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/12107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I774f4a0e2cfedc4dba72cde4f5d28898cdbdc236 Gerrit-Change-Number: 12107 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 19 Dec 2018 20:20:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7213: Use separate network plane for DataStream and Control services
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12107 ) Change subject: IMPALA-7213: Use separate network plane for DataStream and Control services .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3588/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I774f4a0e2cfedc4dba72cde4f5d28898cdbdc236 Gerrit-Change-Number: 12107 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 19 Dec 2018 19:34:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7213: Use separate network plane for DataStream and Control services
Michael Ho has posted comments on this change. (
http://gerrit.cloudera.org:8080/12107 )
Change subject: IMPALA-7213: Use separate network plane for DataStream and
Control services
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12107/1/be/src/service/control-service.cc
File be/src/service/control-service.cc:
http://gerrit.cloudera.org:8080/#/c/12107/1/be/src/service/control-service.cc@83
PS1, Line 83: (*proxy)->set_network_plane("control");
> Maybe add a brief comment about the motivation here
Done
--
To view, visit http://gerrit.cloudera.org:8080/12107
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I774f4a0e2cfedc4dba72cde4f5d28898cdbdc236
Gerrit-Change-Number: 12107
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Thomas Marshall
Gerrit-Comment-Date: Wed, 19 Dec 2018 19:30:40 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7213: Use separate network plane for DataStream and Control services
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12107 ) Change subject: IMPALA-7213: Use separate network plane for DataStream and Control services .. Patch Set 2: Code-Review+2 Carry Thomas' +2. -- To view, visit http://gerrit.cloudera.org:8080/12107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I774f4a0e2cfedc4dba72cde4f5d28898cdbdc236 Gerrit-Change-Number: 12107 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 19 Dec 2018 19:30:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7213: Use separate network plane for DataStream and Control services
Hello Thomas Marshall, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12107 to look at the new patch set (#2). Change subject: IMPALA-7213: Use separate network plane for DataStream and Control services .. IMPALA-7213: Use separate network plane for DataStream and Control services This change is a follow up for a review comment in https://gerrit.cloudera.org/#/c/10855/ about separating the TCP connections of DataStream and Control services so that control commands don't get blocked behind large payloads being sent in the DataStream services. Testing done: exhaustive build Change-Id: I774f4a0e2cfedc4dba72cde4f5d28898cdbdc236 --- M be/src/service/control-service.cc M be/src/service/data-stream-service.cc 2 files changed, 8 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/12107/2 -- To view, visit http://gerrit.cloudera.org:8080/12107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I774f4a0e2cfedc4dba72cde4f5d28898cdbdc236 Gerrit-Change-Number: 12107 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-7265: Add parameter to cache remote HDFS file handles
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12111 Change subject: IMPALA-7265: Add parameter to cache remote HDFS file handles .. IMPALA-7265: Add parameter to cache remote HDFS file handles Currently, the file handle cache does not apply to remote HDFS files. This adds a parameter 'cache_remote_file_handles' that enables the file handle cache for remote HDFS files. It is currently being tested, so it is set to false by default. This does not change the behavior for S3, ADLS, or ABFS. Change-Id: I549f007432a01ca52fa8093d458a220bba02e1d9 --- M be/src/runtime/io/disk-io-mgr.cc M be/src/runtime/io/handle-cache.h M be/src/runtime/io/hdfs-file-reader.cc M be/src/runtime/io/scan-range.cc 4 files changed, 30 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/12111/1 -- To view, visit http://gerrit.cloudera.org:8080/12111 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I549f007432a01ca52fa8093d458a220bba02e1d9 Gerrit-Change-Number: 12111 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell
[Impala-ASF-CR] Add "network plane" as part of ConnectionId
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/12106 ) Change subject: Add "network_plane" as part of ConnectionId .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6767e631fd9530ea54f5ed63ff4c8c179ab216b2 Gerrit-Change-Number: 12106 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 19 Dec 2018 19:05:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5200: Count child time for parent's total time
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11791 ) Change subject: IMPALA-5200: Count child time for parent's total time .. IMPALA-5200: Count child time for parent's total time One problem with the total time counter on runtime profiles is that a parent's time may not be updated if execution is stuck in a child node. The child can accumulate time while the parent is stuck at zero. This leads to incorrect or misleading calculations of total time or non-child time for the parent node during execution. This makes a modest change in calculation for total time for parent nodes. It takes advantage of the fact that the parent should count all of the time from all of its children as total time for itself. Specifically, if a parent has accumulated X in its total timer and its children have accumulated Y summed across all of their timers, then a parent's total time should be at least max(X, Y). There is no way to know the appropriate overlap between X and Y, so this uses a conservative calculation assuming complete overlap. This prevents a parent node from reporting itself as 100% non-child time when it is actually stuck executing child code. However, it does not help if a child node is stuck and is not reporting its own time. Testing: - Added test case to runtime-profile-test - Core tests pass Change-Id: Id6c1191c39fd18b6be45325366a74cf54908c77e Reviewed-on: http://gerrit.cloudera.org:8080/11791 Reviewed-by: Joe McDonnell Tested-by: Impala Public Jenkins --- M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h 3 files changed, 125 insertions(+), 17 deletions(-) Approvals: Joe McDonnell: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Id6c1191c39fd18b6be45325366a74cf54908c77e Gerrit-Change-Number: 11791 Gerrit-PatchSet: 5 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7657: Codegen IsNotEmptyPredicate and ValidTupleIdExpr.
Thomas Marshall has posted comments on this change. (
http://gerrit.cloudera.org:8080/12068 )
Change subject: IMPALA-7657: Codegen IsNotEmptyPredicate and ValidTupleIdExpr.
..
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12068/4/be/src/exprs/is-not-empty-predicate-ir.cc
File be/src/exprs/is-not-empty-predicate-ir.cc:
http://gerrit.cloudera.org:8080/#/c/12068/4/be/src/exprs/is-not-empty-predicate-ir.cc@24
PS4, Line 24: CollectionVal coll = children_[0]->GetCollectionVal(eval, row);
As discussed offline, we should be able to remove this virtual function call
http://gerrit.cloudera.org:8080/#/c/12068/4/be/src/exprs/valid-tuple-id-ir.cc
File be/src/exprs/valid-tuple-id-ir.cc:
http://gerrit.cloudera.org:8080/#/c/12068/4/be/src/exprs/valid-tuple-id-ir.cc@33
PS4, Line 33: for (int i = 0; i < num_tuples; ++i) {
As discussed offline, we should be able to unroll this loop
--
To view, visit http://gerrit.cloudera.org:8080/12068
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb87b9e3b879c278ce8638d97bcb320a7555a6b3
Gerrit-Change-Number: 12068
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Marshall
Gerrit-Comment-Date: Wed, 19 Dec 2018 18:49:52 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7994: Prevent test insert large string from causing OOM issues
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12110 ) Change subject: IMPALA-7994: Prevent test_insert_large_string from causing OOM issues .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1647/ : 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/12110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c950f6860b2f86865dbc5ce60055175e2c0bebc Gerrit-Change-Number: 12110 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 19 Dec 2018 17:55:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7994: Prevent test insert large string from causing OOM issues
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12110 ) Change subject: IMPALA-7994: Prevent test_insert_large_string from causing OOM issues .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c950f6860b2f86865dbc5ce60055175e2c0bebc Gerrit-Change-Number: 12110 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 19 Dec 2018 17:29:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7994: Prevent test insert large string from causing OOM issues
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12110 ) Change subject: IMPALA-7994: Prevent test_insert_large_string from causing OOM issues .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3587/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c950f6860b2f86865dbc5ce60055175e2c0bebc Gerrit-Change-Number: 12110 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 19 Dec 2018 17:29:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7994: Prevent test insert large string from causing OOM issues
Bikramjeet Vig has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12110 Change subject: IMPALA-7994: Prevent test_insert_large_string from causing OOM issues .. IMPALA-7994: Prevent test_insert_large_string from causing OOM issues test_insert_large_string uses upto 4GB of untracked memory which results in random OOMs during exhaustive testing on release builds. Queries run faster on release builds which might result in a different set of tests running together when compared to those on debug builds. This can result in queries requiring more memory running together with test_insert_large_string and eventually encounter OOM errors. Testing: Successfully ran exhaustive tests twice on release build. Change-Id: I6c950f6860b2f86865dbc5ce60055175e2c0bebc --- M tests/query_test/test_insert.py 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/12110/1 -- To view, visit http://gerrit.cloudera.org:8080/12110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6c950f6860b2f86865dbc5ce60055175e2c0bebc Gerrit-Change-Number: 12110 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig
[Impala-ASF-CR] IMPALA-7994: Prevent test insert large string from causing OOM issues
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/12110 ) Change subject: IMPALA-7994: Prevent test_insert_large_string from causing OOM issues .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c950f6860b2f86865dbc5ce60055175e2c0bebc Gerrit-Change-Number: 12110 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 19 Dec 2018 17:26:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12004 ) Change subject: IMPALA-7889: Write new logical types in Parquet .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1646/ : 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/12004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 Gerrit-Change-Number: 12004 Gerrit-PatchSet: 10 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Anonymous Coward (359) Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Zoltan Ivanfi Gerrit-Comment-Date: Wed, 19 Dec 2018 16:46:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/12004 ) Change subject: IMPALA-7889: Write new logical types in Parquet .. Patch Set 10: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 Gerrit-Change-Number: 12004 Gerrit-PatchSet: 10 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Anonymous Coward (359) Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Zoltan Ivanfi Gerrit-Comment-Date: Wed, 19 Dec 2018 16:34:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12004 ) Change subject: IMPALA-7889: Write new logical types in Parquet .. Patch Set 11: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3586/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 Gerrit-Change-Number: 12004 Gerrit-PatchSet: 11 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Anonymous Coward (359) Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Zoltan Ivanfi Gerrit-Comment-Date: Wed, 19 Dec 2018 16:35:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12004 ) Change subject: IMPALA-7889: Write new logical types in Parquet .. Patch Set 11: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 Gerrit-Change-Number: 12004 Gerrit-PatchSet: 11 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Anonymous Coward (359) Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Zoltan Ivanfi Gerrit-Comment-Date: Wed, 19 Dec 2018 16:35:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Anonymous Coward (359) has posted comments on this change. ( http://gerrit.cloudera.org:8080/12004 ) Change subject: IMPALA-7889: Write new logical types in Parquet .. Patch Set 10: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/12004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 Gerrit-Change-Number: 12004 Gerrit-PatchSet: 10 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Anonymous Coward (359) Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Zoltan Ivanfi Gerrit-Comment-Date: Wed, 19 Dec 2018 16:34:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Hello Lars Volker, Anonymous Coward (359), Zoltan Ivanfi, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12004 to look at the new patch set (#10). Change subject: IMPALA-7889: Write new logical types in Parquet .. IMPALA-7889: Write new logical types in Parquet Fill the LogicalType field in Parquet schemas for columns that have an associated logical type. ConvertedType still has to be filled to remain compatible with older readers. Testing: - added new tests to check both logical and converted types to test_insert_parquet.py Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 --- M be/src/exec/parquet/hdfs-parquet-table-writer.cc M be/src/exec/parquet/parquet-metadata-utils.cc M be/src/exec/parquet/parquet-metadata-utils.h M tests/query_test/test_insert_parquet.py M tests/util/get_parquet_metadata.py 5 files changed, 264 insertions(+), 82 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/12004/10 -- To view, visit http://gerrit.cloudera.org:8080/12004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 Gerrit-Change-Number: 12004 Gerrit-PatchSet: 10 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Anonymous Coward (359) Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Zoltan Ivanfi
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Zoltan Ivanfi has posted comments on this change. ( http://gerrit.cloudera.org:8080/12004 ) Change subject: IMPALA-7889: Write new logical types in Parquet .. Patch Set 10: Code-Review+1 LGTM, thanks! -- To view, visit http://gerrit.cloudera.org:8080/12004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 Gerrit-Change-Number: 12004 Gerrit-PatchSet: 10 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Anonymous Coward (359) Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Zoltan Ivanfi Gerrit-Comment-Date: Wed, 19 Dec 2018 16:18:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Csaba Ringhofer has posted comments on this change. (
http://gerrit.cloudera.org:8080/12004 )
Change subject: IMPALA-7889: Write new logical types in Parquet
..
Patch Set 8:
(1 comment)
Patch set 9 is only rebase + conflict resolution while patch set replaces the
"if" chain with a "case" statement.
http://gerrit.cloudera.org:8080/#/c/12004/8/be/src/exec/parquet/parquet-metadata-utils.cc
File be/src/exec/parquet/parquet-metadata-utils.cc:
http://gerrit.cloudera.org:8080/#/c/12004/8/be/src/exec/parquet/parquet-metadata-utils.cc@303
PS8, Line 303: if (col_type.type == TYPE_DECIMAL) {
> (optional) I would prefer a switch-case here.
Done - I moved most logic to separate functions to avoid issues with variable
scopes in "case:" blocks.
--
To view, visit http://gerrit.cloudera.org:8080/12004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91
Gerrit-Change-Number: 12004
Gerrit-PatchSet: 8
Gerrit-Owner: Csaba Ringhofer
Gerrit-Reviewer: Anonymous Coward (359)
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Zoltan Ivanfi
Gerrit-Comment-Date: Wed, 19 Dec 2018 16:15:12 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12004 ) Change subject: IMPALA-7889: Write new logical types in Parquet .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1645/ : 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/12004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 Gerrit-Change-Number: 12004 Gerrit-PatchSet: 9 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Anonymous Coward (359) Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Zoltan Ivanfi Gerrit-Comment-Date: Wed, 19 Dec 2018 16:06:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Hello Lars Volker, Anonymous Coward (359), Zoltan Ivanfi, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12004 to look at the new patch set (#9). Change subject: IMPALA-7889: Write new logical types in Parquet .. IMPALA-7889: Write new logical types in Parquet Fill the LogicalType field in Parquet schemas for columns that have an associated logical type. ConvertedType still has to be filled to remain compatible with older readers. Testing: - added new tests to check both logical and converted types to test_insert_parquet.py Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 --- M be/src/exec/parquet/hdfs-parquet-table-writer.cc M be/src/exec/parquet/parquet-metadata-utils.cc M be/src/exec/parquet/parquet-metadata-utils.h M tests/query_test/test_insert_parquet.py M tests/util/get_parquet_metadata.py 5 files changed, 235 insertions(+), 82 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/12004/9 -- To view, visit http://gerrit.cloudera.org:8080/12004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 Gerrit-Change-Number: 12004 Gerrit-PatchSet: 9 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Anonymous Coward (359) Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Zoltan Ivanfi
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Zoltan Ivanfi has posted comments on this change. (
http://gerrit.cloudera.org:8080/12004 )
Change subject: IMPALA-7889: Write new logical types in Parquet
..
Patch Set 8: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12004/8/be/src/exec/parquet/parquet-metadata-utils.cc
File be/src/exec/parquet/parquet-metadata-utils.cc:
http://gerrit.cloudera.org:8080/#/c/12004/8/be/src/exec/parquet/parquet-metadata-utils.cc@303
PS8, Line 303: if (col_type.type == TYPE_DECIMAL) {
(optional) I would prefer a switch-case here.
--
To view, visit http://gerrit.cloudera.org:8080/12004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91
Gerrit-Change-Number: 12004
Gerrit-PatchSet: 8
Gerrit-Owner: Csaba Ringhofer
Gerrit-Reviewer: Anonymous Coward (359)
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Zoltan Ivanfi
Gerrit-Comment-Date: Wed, 19 Dec 2018 14:51:19 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet
Anonymous Coward (359) has posted comments on this change. ( http://gerrit.cloudera.org:8080/12004 ) Change subject: IMPALA-7889: Write new logical types in Parquet .. Patch Set 8: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/12004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 Gerrit-Change-Number: 12004 Gerrit-PatchSet: 8 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Anonymous Coward (359) Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Wed, 19 Dec 2018 13:32:35 + Gerrit-HasComments: No
