[Impala-ASF-CR] IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10998 ) Change subject: IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics .. Patch Set 14: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 Gerrit-Change-Number: 10998 Gerrit-PatchSet: 14 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 09 Aug 2018 04:33:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10998 ) Change subject: IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics .. IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics Pause monitor: = This commit adds a stripped down version of Hadoop's JvmPauseMonitor class (https://bit.ly/2O6qSwm) . The core implementaion is borrowed from hadoop-common project and the hadoop dependencies are removed. - Removed dependency on AbstractService. - Not relying on Hadoop's Configuration object for reading confs. - Switched to Guava's implementation of Stopwatch. This utility class can detect both GC/non-GC pauses. In case of GC pauses, the GC metrics during the pause period are logged. Sample Output: = Detected pause in JVM or host machine (eg GC): pause of approximately 2356ms GC pool 'PS MarkSweep' had collection(s): count=1 time=2241ms GC pool 'PS Scavenge' had collection(s): count=3 time=352ms Detected pause in JVM or host machine (eg GC): pause of approximately 1964ms GC pool 'PS MarkSweep' had collection(s): count=1 time=2082ms GC pool 'PS Scavenge' had collection(s): count=1 time=251ms Detected pause in JVM or host machine (eg GC): pause of approximately 2120ms GC pool 'PS MarkSweep' had collection(s): count=1 time=2454ms Detected pause in JVM or host machine (eg GC): pause of approximately 2238ms GC pool 'PS MarkSweep' had collection(s): count=5 time=13464ms Detected pause in JVM or host machine (eg GC): pause of approximately 2233ms GC pool 'PS MarkSweep' had collection(s): count=1 time=2733ms JMX Metrics: JMX metrics are now emmitted for Impala and Catalog JVMs at the web end point /jmx. - Impalad: http(s)://:25000/jmx - Catalogd: http(s)://:25020/jmx Misc: Renamed JvmMetric -> JvmMemoryMetric to make the intent more clear. It doesn't relate to the functionality of the patch in anyway. Testing: === - Tested it manually with kill -SIGSTOP/-SIGCONT . Made sure that the non-GC JVM pauses are logged. - This class' functionality is tested manually by invoking it's main() - Injected a memory leak into the Catalog server code and made sure the GC is detected. Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 Reviewed-on: http://gerrit.cloudera.org:8080/10998 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/common/init.cc M be/src/service/impala-http-handler.cc M be/src/util/default-path-handlers.cc M be/src/util/jni-util.cc M be/src/util/jni-util.h M be/src/util/memory-metrics.cc M be/src/util/memory-metrics.h M be/src/util/metrics-test.cc M be/src/util/webserver-test.cc M be/src/util/webserver.cc M be/src/util/webserver.h M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/common/JniUtil.java A fe/src/main/java/org/apache/impala/util/JMXJsonUtil.java A fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java A fe/src/test/java/org/apache/impala/util/JMXJsonUtilTest.java A fe/src/test/java/org/apache/impala/util/JniUtilTest.java A tests/custom_cluster/test_pause_monitor.py M tests/webserver/test_web_pages.py 19 files changed, 850 insertions(+), 90 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/10998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 Gerrit-Change-Number: 10998 Gerrit-PatchSet: 15 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6034: Add scanned bytes limits per query
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11081 ) Change subject: IMPALA-6034: Add scanned bytes limits per query .. Patch Set 3: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2964/ -- To view, visit http://gerrit.cloudera.org:8080/11081 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3e85f80b70b3fce47e637e9322ed0316ee84f6a9 Gerrit-Change-Number: 11081 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 09 Aug 2018 04:08:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7388. Fix issues in and optimize various Status macros
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11153 ) Change subject: IMPALA-7388. Fix issues in and optimize various Status macros .. Patch Set 4: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2963/ -- To view, visit http://gerrit.cloudera.org:8080/11153 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3d62c99bbd83cc290f5992077258af156eaa6330 Gerrit-Change-Number: 11153 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 09 Aug 2018 03:28:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10855 ) Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/264/ : 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/10855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe Gerrit-Change-Number: 10855 Gerrit-PatchSet: 10 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 09 Aug 2018 03:20:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7361: Fix flakiness in test heterogeneous proc mem limit
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11166 ) Change subject: IMPALA-7361: Fix flakiness in test_heterogeneous_proc_mem_limit .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11166 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3e5c9b0c6a99d7157640c02e6b3c808b4ae9e73c Gerrit-Change-Number: 11166 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 09 Aug 2018 03:04:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7361: Fix flakiness in test heterogeneous proc mem limit
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11166 ) Change subject: IMPALA-7361: Fix flakiness in test_heterogeneous_proc_mem_limit .. IMPALA-7361: Fix flakiness in test_heterogeneous_proc_mem_limit This patch fixes flakiness in test_heterogeneous_proc_mem_limit wherein one of the fragments on a host from a previous query lingers and holds on to resources which causes the next query to fail the test since it expects the cluster to be idle at that point. Change-Id: I3e5c9b0c6a99d7157640c02e6b3c808b4ae9e73c Reviewed-on: http://gerrit.cloudera.org:8080/11166 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M tests/custom_cluster/test_admission_controller.py 1 file changed, 3 insertions(+), 0 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11166 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3e5c9b0c6a99d7157640c02e6b3c808b4ae9e73c Gerrit-Change-Number: 11166 Gerrit-PatchSet: 3 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10855 ) Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC .. Patch Set 10: (3 comments) http://gerrit.cloudera.org:8080/#/c/10855/9/be/src/runtime/coordinator-backend-state.h File be/src/runtime/coordinator-backend-state.h: http://gerrit.cloudera.org:8080/#/c/10855/9/be/src/runtime/coordinator-backend-state.h@33 PS9, Line 33: #include "runtime/coordina > Not needed. Done http://gerrit.cloudera.org:8080/#/c/10855/9/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/10855/9/be/src/runtime/coordinator-backend-state.cc@52 PS9, Line 52: > This should have been moved to line 48. Done http://gerrit.cloudera.org:8080/#/c/10855/9/common/protobuf/control_service.proto File common/protobuf/control_service.proto: http://gerrit.cloudera.org:8080/#/c/10855/9/common/protobuf/control_service.proto@137 PS9, Line 137: 5 > Skipped 5. Done -- To view, visit http://gerrit.cloudera.org:8080/10855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe Gerrit-Change-Number: 10855 Gerrit-PatchSet: 10 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 09 Aug 2018 02:46:41 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC
Hello Sailesh Mukil, Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10855 to look at the new patch set (#10). Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC .. IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC This change converts ReportExecStatus() RPC from thrift based RPC to KRPC. This is done in part of the preparation for fixing IMPALA-2990 as we can take advantage of TCP connection multiplexing in KRPC to avoid overwhelming the coordinator with too many connections by reducing the number of TCP connection to one for each executor. This patch also introduces a new service pool for all query execution control related RPCs in the future so that control commands from coordinators aren't blocked by long-running DataStream services' RPCs. The majority of this patch is mechanical conversion of some Thrift structures used in ReportExecStatus() RPC to Protobuf. Note that the runtime profile is still retained as a Thrift structure as Impala clients will still fetch query profiles using Thrift RPCs. This also avoids duplicating the serialization implementation in both Thrift and Protobuf for the runtime profile. The Thrift runtime profiles are serialized and sent as a sidecar in ReportExecStatus() RPC. This patch also fixes IMPALA-7241 which may lead to duplicated dml stats being applied. The fix is by adding a monotonically increasing version number for fragment instances' reports. The coordinator will ignore any report smaller than or equal to the version in the last report. Testing done: core build. Added some targeted test cases for profile serialization failure and RPC retry. Change-Id: I7638583b433dcac066b87198e448743d90415ebe --- M be/src/benchmarks/expr-benchmark.cc M be/src/catalog/catalog-util.cc M be/src/exec/data-sink.cc M be/src/exec/data-sink.h M be/src/exec/hbase-table-sink.cc M be/src/exec/hdfs-parquet-table-writer.cc M be/src/exec/hdfs-parquet-table-writer.h M be/src/exec/hdfs-table-sink.cc M be/src/exec/hdfs-table-writer.cc M be/src/exec/hdfs-table-writer.h M be/src/rpc/jni-thrift-util.h M be/src/rpc/thrift-util-test.cc M be/src/rpc/thrift-util.h M be/src/runtime/backend-client.h M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-backend-state.h M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/dml-exec-state.cc M be/src/runtime/dml-exec-state.h M be/src/runtime/exec-env.cc M be/src/runtime/exec-env.h M be/src/runtime/fragment-instance-state.cc M be/src/runtime/fragment-instance-state.h M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M be/src/runtime/test-env.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/scheduler-test-util.cc M be/src/service/CMakeLists.txt M be/src/service/client-request-state.cc M be/src/service/client-request-state.h A be/src/service/control-service.cc A be/src/service/control-service.h M be/src/service/data-stream-service.cc M be/src/service/data-stream-service.h M be/src/service/impala-internal-service.cc M be/src/service/impala-internal-service.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/testutil/in-process-servers.cc M be/src/util/container-util.h A be/src/util/error-util-internal.h M be/src/util/error-util-test.cc M be/src/util/error-util.cc M be/src/util/error-util.h M be/src/util/runtime-profile.cc M be/src/util/uid-util.h M bin/impala-config.sh M common/protobuf/CMakeLists.txt M common/protobuf/common.proto A common/protobuf/control_service.proto M common/protobuf/data_stream_service.proto M common/protobuf/row_batch.proto M common/protobuf/rpc_test.proto M common/thrift/ImpalaInternalService.thrift M tests/custom_cluster/test_rpc_exception.py M tests/failure/test_failpoints.py 60 files changed, 1,110 insertions(+), 673 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/10855/10 -- To view, visit http://gerrit.cloudera.org:8080/10855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe Gerrit-Change-Number: 10855 Gerrit-PatchSet: 10 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10855 ) Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/10855/9/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/10855/9/be/src/runtime/coordinator-backend-state.cc@52 PS9, Line 52: DCHECK_NOTNULL(coord); This should have been moved to line 48. -- To view, visit http://gerrit.cloudera.org:8080/10855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe Gerrit-Change-Number: 10855 Gerrit-PatchSet: 9 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 09 Aug 2018 01:49:54 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7415: Fix flakiness in test multiline queries in history
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11175 ) Change subject: IMPALA-7415: Fix flakiness in test_multiline_queries_in_history .. Patch Set 1: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/263/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/11175 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If7474f832a88bc29b321f21b050c9665294e63d5 Gerrit-Change-Number: 11175 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 09 Aug 2018 01:48:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7096: restore scanner thread memory heuristics
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11103 ) Change subject: IMPALA-7096: restore scanner thread memory heuristics .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9907fa8c4d2b0b85f67f4f160899c1c258ad82b Gerrit-Change-Number: 11103 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 09 Aug 2018 01:32:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7335: Add logs HdfsScanNode to debug the issue
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11174 ) Change subject: IMPALA-7335: Add logs HdfsScanNode to debug the issue .. Patch Set 1: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/262/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/11174 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I68698c90031edc6ee8c31e9ce3d52dade9d8f6f1 Gerrit-Change-Number: 11174 Gerrit-PatchSet: 1 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 09 Aug 2018 01:25:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7383: Configurable HMS and Sentry policy DB
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11104 ) Change subject: IMPALA-7383: Configurable HMS and Sentry policy DB .. Patch Set 6: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/261/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/11104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I190d657cb95dfdf73ebd05e5dd24ef2a8e3156b8 Gerrit-Change-Number: 11104 Gerrit-PatchSet: 6 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Comment-Date: Thu, 09 Aug 2018 01:20:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10998 ) Change subject: IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics .. Patch Set 14: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2966/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 Gerrit-Change-Number: 10998 Gerrit-PatchSet: 14 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 09 Aug 2018 01:20:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10998 ) Change subject: IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics .. Patch Set 14: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 Gerrit-Change-Number: 10998 Gerrit-PatchSet: 14 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 09 Aug 2018 01:20:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7335: Add logs HdfsScanNode to debug the issue
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/11174 ) Change subject: IMPALA-7335: Add logs HdfsScanNode to debug the issue .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11174/1/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/11174/1/be/src/exec/hdfs-scan-node.cc@331 PS1, Line 331: status Might help to print this out too just to see if it's CANCELLED or something else. -- To view, visit http://gerrit.cloudera.org:8080/11174 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I68698c90031edc6ee8c31e9ce3d52dade9d8f6f1 Gerrit-Change-Number: 11174 Gerrit-PatchSet: 1 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 09 Aug 2018 01:15:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7415: Fix flakiness in test multiline queries in history
Bikramjeet Vig has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11175 Change subject: IMPALA-7415: Fix flakiness in test_multiline_queries_in_history .. IMPALA-7415: Fix flakiness in test_multiline_queries_in_history This fixes a flakiness in test_multiline_queries_in_history wherein a part of the shell prompt would be absorbed in a previous regex search that would ultimately result in the failure of the subsequent regex search that looks for the prompt. Also fixed a few formatting issues flagged by flake8. Change-Id: If7474f832a88bc29b321f21b050c9665294e63d5 --- M tests/shell/test_shell_interactive.py 1 file changed, 12 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/11175/1 -- To view, visit http://gerrit.cloudera.org:8080/11175 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: If7474f832a88bc29b321f21b050c9665294e63d5 Gerrit-Change-Number: 11175 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig
[Impala-ASF-CR] IMPALA-7335: Add logs HdfsScanNode to debug the issue
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11174 ) Change subject: IMPALA-7335: Add logs HdfsScanNode to debug the issue .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11174/1/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/11174/1/be/src/exec/hdfs-scan-node.cc@334 PS1, Line 334: << ", done_ = " << (done_ ? "1" : "0") << ")"; tab used for whitespace -- To view, visit http://gerrit.cloudera.org:8080/11174 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I68698c90031edc6ee8c31e9ce3d52dade9d8f6f1 Gerrit-Change-Number: 11174 Gerrit-PatchSet: 1 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 09 Aug 2018 01:12:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7335: Add logs HdfsScanNode to debug the issue
Pooja Nilangekar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11174 Change subject: IMPALA-7335: Add logs HdfsScanNode to debug the issue .. IMPALA-7335: Add logs HdfsScanNode to debug the issue This log would help determine the condition when the test failures occur. Change-Id: I68698c90031edc6ee8c31e9ce3d52dade9d8f6f1 --- M be/src/exec/hdfs-scan-node.cc 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/11174/1 -- To view, visit http://gerrit.cloudera.org:8080/11174 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I68698c90031edc6ee8c31e9ce3d52dade9d8f6f1 Gerrit-Change-Number: 11174 Gerrit-PatchSet: 1 Gerrit-Owner: Pooja Nilangekar
[Impala-ASF-CR] IMPALA-6153: Execute UpdateFilter() only for executing queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 ) Change subject: IMPALA-6153: Execute UpdateFilter() only for executing queries .. Patch Set 10: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c Gerrit-Change-Number: 11005 Gerrit-PatchSet: 10 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 09 Aug 2018 01:04:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6153: Execute UpdateFilter() only for executing queries
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11005 ) Change subject: IMPALA-6153: Execute UpdateFilter() only for executing queries .. IMPALA-6153: Execute UpdateFilter() only for executing queries This change ensures that coordinator::UpdateFilter is executed only for queries which are in the EXECUTING state. Additionally, it also guarantees that coordinator::ReleaseExecResources is executed only when no other thread is executing UpdateFilter. Testing: Ran all back-end tests. Additionally, ran the exhaustive tests with DCHECKs which would be triggered if the ReleaseExecResources() is executed while any other thread is executing UpdateFilter(). Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c Reviewed-on: http://gerrit.cloudera.org:8080/11005 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h 2 files changed, 45 insertions(+), 16 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c Gerrit-Change-Number: 11005 Gerrit-PatchSet: 11 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10855 ) Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC .. Patch Set 9: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/260/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/10855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe Gerrit-Change-Number: 10855 Gerrit-PatchSet: 9 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 09 Aug 2018 01:03:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7320. Avoid calling getFileStatus() for each partition when table is loaded
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11027 ) Change subject: IMPALA-7320. Avoid calling getFileStatus() for each partition when table is loaded .. Patch Set 8: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11027 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83e5ebc214d6620d165e13f8cc80f8fdda100734 Gerrit-Change-Number: 11027 Gerrit-PatchSet: 8 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 09 Aug 2018 00:52:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7383: Configurable HMS and Sentry policy DB
Tianyi Wang has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/11104 ) Change subject: IMPALA-7383: Configurable HMS and Sentry policy DB .. IMPALA-7383: Configurable HMS and Sentry policy DB Some developers keep multiple impala repos on their disk. Isolating METASTORE_DB and SENTRY_POLICY_DB may help with switching between those repos without reloading the data. This patch makes those DB names configurable and default to an escaped IMPALA_HOME path. Change-Id: I190d657cb95dfdf73ebd05e5dd24ef2a8e3156b8 --- M bin/create-test-configuration.sh M bin/impala-config.sh M fe/src/test/resources/sentry-site.xml.template 3 files changed, 6 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/11104/6 -- To view, visit http://gerrit.cloudera.org:8080/11104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I190d657cb95dfdf73ebd05e5dd24ef2a8e3156b8 Gerrit-Change-Number: 11104 Gerrit-PatchSet: 6 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang
[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10855 ) Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/query-state.cc File be/src/runtime/query-state.cc: http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/query-state.cc@373 PS8, Line 373: MonoDelta::FromMilliseconds(FLAGS_backend_client_rpc_timeout_ms)); > We have some logging already below at line 385. Actually, mis-interpreted your comment. Will add a LOG(WARNING). -- To view, visit http://gerrit.cloudera.org:8080/10855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe Gerrit-Change-Number: 10855 Gerrit-PatchSet: 9 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 09 Aug 2018 00:49:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6034: Add scanned bytes limits per query
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11081 ) Change subject: IMPALA-6034: Add scanned bytes limits per query .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2964/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11081 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3e85f80b70b3fce47e637e9322ed0316ee84f6a9 Gerrit-Change-Number: 11081 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 09 Aug 2018 00:43:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6034: Add scanned bytes limits per query
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11081 ) Change subject: IMPALA-6034: Add scanned bytes limits per query .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11081 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3e85f80b70b3fce47e637e9322ed0316ee84f6a9 Gerrit-Change-Number: 11081 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 09 Aug 2018 00:43:23 + Gerrit-HasComments: No
[Impala-ASF-CR] Exclude files with long mangled symbols from whitespace checks
Tim Armstrong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11169 ) Change subject: Exclude files with long mangled symbols from whitespace checks .. Exclude files with long mangled symbols from whitespace checks Change-Id: Ia99cfc9840cc0d80d2654089f7e776344831a6b2 Reviewed-on: http://gerrit.cloudera.org:8080/11169 Reviewed-by: Philip Zeyliger Tested-by: Tim Armstrong --- M bin/jenkins/critique-gerrit-review.py 1 file changed, 2 insertions(+), 0 deletions(-) Approvals: Philip Zeyliger: Looks good to me, approved Tim Armstrong: Verified -- To view, visit http://gerrit.cloudera.org:8080/11169 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ia99cfc9840cc0d80d2654089f7e776344831a6b2 Gerrit-Change-Number: 11169 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Exclude files with long mangled symbols from whitespace checks
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11169 ) Change subject: Exclude files with long mangled symbols from whitespace checks .. Patch Set 2: Verified+1 Manually tested locally - not exercised by precommit. -- To view, visit http://gerrit.cloudera.org:8080/11169 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia99cfc9840cc0d80d2654089f7e776344831a6b2 Gerrit-Change-Number: 11169 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 09 Aug 2018 00:36:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7410: fix centos6 dependency
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11168 ) Change subject: IMPALA-7410: fix centos6 dependency .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11168 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0fd2ddac5d818a0118c74947532b773e0733 Gerrit-Change-Number: 11168 Gerrit-PatchSet: 2 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 09 Aug 2018 00:34:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7410: fix centos6 dependency
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11168 ) Change subject: IMPALA-7410: fix centos6 dependency .. IMPALA-7410: fix centos6 dependency Update CDH_BUILD_NUM to address incorrect dependency for centos6. Change-Id: I0fd2ddac5d818a0118c74947532b773e0733 Reviewed-on: http://gerrit.cloudera.org:8080/11168 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M bin/impala-config.sh 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11168 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I0fd2ddac5d818a0118c74947532b773e0733 Gerrit-Change-Number: 11168 Gerrit-PatchSet: 3 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10855 ) Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC .. Patch Set 9: (2 comments) http://gerrit.cloudera.org:8080/#/c/10855/9/be/src/runtime/coordinator-backend-state.h File be/src/runtime/coordinator-backend-state.h: http://gerrit.cloudera.org:8080/#/c/10855/9/be/src/runtime/coordinator-backend-state.h@33 PS9, Line 33: #include "common/atomic.h" Not needed. http://gerrit.cloudera.org:8080/#/c/10855/9/common/protobuf/control_service.proto File common/protobuf/control_service.proto: http://gerrit.cloudera.org:8080/#/c/10855/9/common/protobuf/control_service.proto@137 PS9, Line 137: 6 Skipped 5. -- To view, visit http://gerrit.cloudera.org:8080/10855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe Gerrit-Change-Number: 10855 Gerrit-PatchSet: 9 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 09 Aug 2018 00:22:23 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10855 ) Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC .. Patch Set 9: (1 comment) just responding to the comment. will review the new rev later tonight http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.cc@262 PS8, Line 262: ProtoToQueryId(instance_exec_status.fragment_instance_id())); > The reason for doing it here is to handle the case in which we can have 0 t OK. I'm still not 100% convinced that the sidecars even matter in the first place (just eliminating a copy of a few MB which probably takes on the order of tens of us or less), but the solution you proposed seems to make sense if we take sidecar usage as a requirement. -- To view, visit http://gerrit.cloudera.org:8080/10855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe Gerrit-Change-Number: 10855 Gerrit-PatchSet: 9 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 09 Aug 2018 00:19:03 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7409. CatalogObjectVersionSet should not allow duplicates
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11151 ) Change subject: IMPALA-7409. CatalogObjectVersionSet should not allow duplicates .. Patch Set 2: > Patch Set 2: Verified-1 > > Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2960/ Our fears were true -- we are misusing this data structure in various spots. Guess we'll need to verify our assumptions and see if our code or our understanding of the code is at fault :) -- To view, visit http://gerrit.cloudera.org:8080/11151 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I318e59c6a7fe559e86693fce1255fe6d9f829ccf Gerrit-Change-Number: 11151 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 09 Aug 2018 00:15:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10855 ) Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC .. Patch Set 8: (16 comments) http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/exec/data-sink.cc File be/src/exec/data-sink.cc: http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/exec/data-sink.cc@49 PS8, Line 49: const char* DataSink::ROOT_PARTITION_KEY = ""; > this should be const char* const -- ie it's a const pointer to const charac True. Same comment probably applies to other classes too (e.g. https://github.com/apache/impala/blob/master/be/src/exec/hash-table.h#L196) http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/exec/hdfs-table-writer.h File be/src/exec/hdfs-table-writer.h: http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/exec/hdfs-table-writer.h@91 PS8, Line 91: const InsertStatsPB& stats() { return stats_; } > probably should be a const function as well? Done http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.h File be/src/runtime/coordinator-backend-state.h: http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.h@211 PS8, Line 211: Coordinator* coord_; /// Coordinator object that owns this BackendState > this went from const to non-const. does this mutate the coordinator that ow Indirectly. We are updating the coord_->dml_exec_state_(). The locking is done inside dml_exec_state_ itself so it should be thread safe. http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.cc@50 PS8, Line 50: : coord_(coord), > nit: you could just do coord_(DHECK_NOTNULL(coord)) here since DCHECK_NOTNU Done http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.cc@262 PS8, Line 262: lock.unlock(); > I'm nervous about this optimization. Do we really have data that deserializ The reason for doing it here is to handle the case in which we can have 0 to # fragment instances (after IMPALA-4063) entries in "backend_exec_status. instance_exec_status". We can have 0 entries if say we failed to start a thread. So, while we can do it in the RPC handler, it's a bit awkward to handle the zero entry case. The new patch moves the profile sidecar idx into backend_exec_status from backend_exec_status.instance_exec_status. That allows us to keep the deserialization in the ControlService::ReportExecStatus(). In the patch for IMPALA-4063, we will just replace TRuntimeProfileTree with a list instead. http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.cc@310 PS8, Line 310: coord_ > I feel that allowing for a non-const pointer to the Coordinator object to t Done http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.cc@317 PS8, Line 317: MergeErrorMaps(instance_exec_status.error_log(), &error_log_); > This is less severe, but I guess that this is already wrong too. We potenti The new report has a monotonically increasing version number. This is a variant of IMPALA-7241. http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.cc@516 PS8, Line 516: lock_guard l1(exec_summary->lock); > Are we violating any lock ordering here? Yes, I also looked and didn't see anything. http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/query-state.cc File be/src/runtime/query-state.cc: http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/query-state.cc@318 PS8, Line 318: bool done > trying to understand the purpose of this parameter. I don't think this shou I agree in general. Let me add a TODO. http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/query-state.cc@331 PS8, Line 331: ThriftSerializer serializer(true); > Why construct this here and pass it into ConstructReport? It seems Construc The serialization output is owned by ThriftSerializer. My mistake to use a string below. I was using (buffer, len) earlier to avoid the extra copying. Also added comments about the lifetime of the buffer. http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/query-state.cc@359 PS8, Line 359: CHECK(sidecar_status.ok()) : << FromKuduStatus(sidecar_status, "Failed to add sidecar").GetDetail(); > is there any chance that the profile would extend past max RPC size? I gues Added a CHECK for it in ConstructReport(). http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/query-state.cc@373 PS8, Line 373: if (i < 2) SleepForMs(FLAGS_report_status_retry_interval_ms); > worth a LOG(WARNING) here? We have some logging already below at line 385. http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/service/control-service.cc File be/src/service/control-service.cc: http://
[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC
Hello Sailesh Mukil, Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10855 to look at the new patch set (#9). Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC .. IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC This change converts ReportExecStatus() RPC from thrift based RPC to KRPC. This is done in part of the preparation for fixing IMPALA-2990 as we can take advantage of TCP connection multiplexing in KRPC to avoid overwhelming the coordinator with too many connections by reducing the number of TCP connection to one for each executor. This patch also introduces a new service pool for all query execution control related RPCs in the future so that control commands from coordinators aren't blocked by long-running DataStream services' RPCs. The majority of this patch is mechanical conversion of some Thrift structures used in ReportExecStatus() RPC to Protobuf. Note that the runtime profile is still retained as a Thrift structure as Impala clients will still fetch query profiles using Thrift RPCs. This also avoids duplicating the serialization implementation in both Thrift and Protobuf for the runtime profile. The Thrift runtime profiles are serialized and sent as a sidecar in ReportExecStatus() RPC. This patch also fixes IMPALA-7241 which may lead to duplicated dml stats being applied. The fix is by adding a monotonically increasing version number for fragment instances' reports. The coordinator will ignore any report smaller than or equal to the version in the last report. Testing done: core build. Added some targeted test cases for profile serialization failure and RPC retry. Change-Id: I7638583b433dcac066b87198e448743d90415ebe --- M be/src/benchmarks/expr-benchmark.cc M be/src/catalog/catalog-util.cc M be/src/exec/data-sink.cc M be/src/exec/data-sink.h M be/src/exec/hbase-table-sink.cc M be/src/exec/hdfs-parquet-table-writer.cc M be/src/exec/hdfs-parquet-table-writer.h M be/src/exec/hdfs-table-sink.cc M be/src/exec/hdfs-table-writer.cc M be/src/exec/hdfs-table-writer.h M be/src/rpc/jni-thrift-util.h M be/src/rpc/thrift-util-test.cc M be/src/rpc/thrift-util.h M be/src/runtime/backend-client.h M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-backend-state.h M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/dml-exec-state.cc M be/src/runtime/dml-exec-state.h M be/src/runtime/exec-env.cc M be/src/runtime/exec-env.h M be/src/runtime/fragment-instance-state.cc M be/src/runtime/fragment-instance-state.h M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M be/src/runtime/test-env.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/scheduler-test-util.cc M be/src/service/CMakeLists.txt M be/src/service/client-request-state.cc M be/src/service/client-request-state.h A be/src/service/control-service.cc A be/src/service/control-service.h M be/src/service/data-stream-service.cc M be/src/service/data-stream-service.h M be/src/service/impala-internal-service.cc M be/src/service/impala-internal-service.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/testutil/in-process-servers.cc M be/src/util/container-util.h A be/src/util/error-util-internal.h M be/src/util/error-util-test.cc M be/src/util/error-util.cc M be/src/util/error-util.h M be/src/util/runtime-profile.cc M be/src/util/uid-util.h M bin/impala-config.sh M common/protobuf/CMakeLists.txt M common/protobuf/common.proto A common/protobuf/control_service.proto M common/protobuf/data_stream_service.proto M common/protobuf/row_batch.proto M common/protobuf/rpc_test.proto M common/thrift/ImpalaInternalService.thrift M tests/custom_cluster/test_rpc_exception.py M tests/failure/test_failpoints.py 60 files changed, 1,110 insertions(+), 673 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/10855/9 -- To view, visit http://gerrit.cloudera.org:8080/10855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe Gerrit-Change-Number: 10855 Gerrit-PatchSet: 9 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7388. Fix issues in and optimize various Status macros
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11153 ) Change subject: IMPALA-7388. Fix issues in and optimize various Status macros .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2963/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11153 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3d62c99bbd83cc290f5992077258af156eaa6330 Gerrit-Change-Number: 11153 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 09 Aug 2018 00:14:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-110: Support for multiple DISTINCT
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10771 ) Change subject: IMPALA-110: Support for multiple DISTINCT .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/streaming-aggregation-node.cc File be/src/exec/streaming-aggregation-node.cc: http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/streaming-aggregation-node.cc@79 PS4, Line 79: for (int i = 0; i < aggs_.size(); ++i) { > It might be worth adding logic like curr_agg_idx_ in AggregationNode to lin *avoid linear search -- To view, visit http://gerrit.cloudera.org:8080/10771 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I055402eaef6d81e5f70e850d9f8a621e766830a4 Gerrit-Change-Number: 10771 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 09 Aug 2018 00:13:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-110: Support for multiple DISTINCT
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10771 ) Change subject: IMPALA-110: Support for multiple DISTINCT .. Patch Set 4: (29 comments) I did a pass over the backend and the tests. It looks solid to me, comments are mainly aimed at making it more readable for people in the future and catching regressions. I'd like to understand the fe part too. http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/aggregation-node-base.h File be/src/exec/aggregation-node-base.h: http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/aggregation-node-base.h@43 PS4, Line 43: bool replicate_input_; const http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/aggregation-node.h File be/src/exec/aggregation-node.h: http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/aggregation-node.h@50 PS4, Line 50: curr_agg_idx_ optional: maybe something more explicit like curr_output_agg_idx_? http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/aggregation-node.cc File be/src/exec/aggregation-node.cc: http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/aggregation-node.cc@49 PS4, Line 49: batch maybe child_batch or input_batch to disambiguate from the minibatches http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/aggregation-node.cc@55 PS4, Line 55: new RowBatch(child(0)->row_desc(), state->batch_size(), mem_tracker())); It's a little unfortunate that there's no way in C++ apparently to forward arguments to an array constructor, otherwise we could just use unique_ptr http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/aggregation-node.cc@56 PS4, Line 56: mini_batches.push_back(std::move(mini_batch)); nit: might be possible to make this more concise with something like: mini_batches.push_back(make_unique(child(0)->row_desc(), state->batch_size(), mem_tracker(; http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/aggregator.h File be/src/exec/aggregator.h: http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/aggregator.h@81 PS4, Line 81: /// Used to insert input rows if this is a streaming pre-agg. Tries to aggregate all of nit: add blank lines between methods with comments (existing code violated this style) http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/aggregator.h@93 PS4, Line 93: virtual bool eos() = 0; This is a bit pedantic (so I'm ok if you ignore) but I think eos() and num_grouping_exprs() should be Eos() and GetNumGroupingExprs() according to the style guide since they're not really accessors: https://google.github.io/styleguide/cppguide.html#Function_Names Or we could add a eos_ member and make this a true accessor. http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/aggregator.h@110 PS4, Line 110: int agg_idx_; const? we're not that consistent about doing this in the code but I find it helpful to understand the lifetime of members. http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/grouping-aggregator.h File be/src/exec/grouping-aggregator.h: http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/grouping-aggregator.h@295 PS4, Line 295: input "input batch" http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/grouping-aggregator.h@296 PS4, Line 296: /// is fully processed, 'streaming_eos_' will be true. Might be worth mentioning that these are specifically required for the multi-aggregator case. http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/grouping-aggregator.h@298 PS4, Line 298: bool streaming_eos_; Could we eliminate streaming_eos_ if we used streaming_idx_ == in_batch->num_rows() to mean the same thing? It seems to fit. http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/grouping-aggregator.cc File be/src/exec/grouping-aggregator.cc: http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/grouping-aggregator.cc@118 PS4, Line 118: streaming_idx_(0), consider initialising these in the header - i personally find it more readable. http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/streaming-aggregation-node.h File be/src/exec/streaming-aggregation-node.h: http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/streaming-aggregation-node.h@69 PS4, Line 69: int32_t replicate_agg_idx_; These need comments. child_batch_eos_ is also potentially confusing. Wonder if there's a better name, e.g. 'child_batch_processing_done_' or something more elegant than that. http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/streaming-aggregation-node.cc File be/src/exec/streaming-aggregation-node.cc: http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/streaming-aggregation-node.cc@37 PS4, Line 37: child_eos_(false), Same comment about initializing the constants in the header. http://gerrit.cloudera.org:8080/#/c/10771/4/be/src/exec/streaming-aggregation-node.cc@79 PS4, Line 79: for (int i = 0; i < aggs_.size(); ++i) { It might be worth adding logic like curr_
[Impala-ASF-CR] IMPALA-7388. Fix issues in and optimize various Status macros
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/11153 ) Change subject: IMPALA-7388. Fix issues in and optimize various Status macros .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11153 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3d62c99bbd83cc290f5992077258af156eaa6330 Gerrit-Change-Number: 11153 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 09 Aug 2018 00:10:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7361: Fix flakiness in test heterogeneous proc mem limit
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11166 ) Change subject: IMPALA-7361: Fix flakiness in test_heterogeneous_proc_mem_limit .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11166 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3e5c9b0c6a99d7157640c02e6b3c808b4ae9e73c Gerrit-Change-Number: 11166 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 23:50:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7361: Fix flakiness in test heterogeneous proc mem limit
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11166 ) Change subject: IMPALA-7361: Fix flakiness in test_heterogeneous_proc_mem_limit .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2962/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11166 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3e5c9b0c6a99d7157640c02e6b3c808b4ae9e73c Gerrit-Change-Number: 11166 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 23:50:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7361: Fix flakiness in test heterogeneous proc mem limit
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11166 ) Change subject: IMPALA-7361: Fix flakiness in test_heterogeneous_proc_mem_limit .. Patch Set 1: Code-Review+2 GVO failed due to unrelated flaky test. Filed a jira for that IMPALA-7415 -- To view, visit http://gerrit.cloudera.org:8080/11166 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3e5c9b0c6a99d7157640c02e6b3c808b4ae9e73c Gerrit-Change-Number: 11166 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 23:50:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7388. Fix issues in and optimize various Status macros
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11153 ) Change subject: IMPALA-7388. Fix issues in and optimize various Status macros .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/259/ : 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/11153 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3d62c99bbd83cc290f5992077258af156eaa6330 Gerrit-Change-Number: 11153 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 08 Aug 2018 23:45:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7231: group plan nodes into pipelines
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/10848 ) Change subject: IMPALA-7231: group plan nodes into pipelines .. Patch Set 2: Code-Review+2 I think this makes sense. It makes sense to get it in. -- To view, visit http://gerrit.cloudera.org:8080/10848 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1d10eb14d997242f445e5c5fc5362d5410370721 Gerrit-Change-Number: 10848 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 23:38:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7409. CatalogObjectVersionSet should not allow duplicates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11151 ) Change subject: IMPALA-7409. CatalogObjectVersionSet should not allow duplicates .. Patch Set 2: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2960/ -- To view, visit http://gerrit.cloudera.org:8080/11151 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I318e59c6a7fe559e86693fce1255fe6d9f829ccf Gerrit-Change-Number: 11151 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 08 Aug 2018 23:32:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7388. Fix issues in and optimize various Status macros
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11153 ) Change subject: IMPALA-7388. Fix issues in and optimize various Status macros .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/11153/3/be/src/common/status.h File be/src/common/status.h: http://gerrit.cloudera.org:8080/#/c/11153/3/be/src/common/status.h@291 PS3, Line 291: const ::impala::Status& _status = (stmt); \ > While you're here, maybe consider fixing these names. From the standard's [ Done, at least in the areas this patch touches. There are a few others lurking but didn't want to expact scope of this patch to other areas. -- To view, visit http://gerrit.cloudera.org:8080/11153 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3d62c99bbd83cc290f5992077258af156eaa6330 Gerrit-Change-Number: 11153 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 08 Aug 2018 23:16:09 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7388. Fix issues in and optimize various Status macros
Hello Jim Apple, Tim Armstrong, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11153 to look at the new patch set (#4). Change subject: IMPALA-7388. Fix issues in and optimize various Status macros .. IMPALA-7388. Fix issues in and optimize various Status macros Prior to this patch, the JNI THROW_IF macros used a variable 'status' in local scope to hold the result of the statement whose status is to be checked. This could cause problems if there is another variable 'status' in the external scope, as the inner scope would shadow the outer. For example: Status status = foo(); THROW_IF_ERROR(status, ...); ... would actually end up resulting in a macro expansion like: do { Status status = status; ... } while (false); Suprisingly, such code is legal C++ and ends up calling the Status copy constructor with an uninitialized version of itself as the argument. gcc doesn't seem to emit a warning for this construct, though clang does emit "variable 'status' is uninitialized when used within its own initialization". If such code is emitted, we'll get undefined behavior and most likely not end up throwing an error even if status is not OK. The fix is simple: use a less-likely-to-conflict variable name _status in the macro. This also fixes other macros that previously used __status__ to use _status to align with the fact that __*__ style keywords are reserved by the C++ standard. Additionally, this patch changes some other similar macros to use const references instead of creating new variables. On the surface, it might look wrong to assign a const reference to a temporary such as the result of a function call. However, C++ explicitly allows this and ensures that the lifetime of the temporary is extended to the lifetime of the captured reference, making it safe[1]. This acts as a small optimization since the RETURN_IF_ERROR macros won't call their copy-constructor an extra time. [1] https://herbsutter.com/2008/01/01/gotw-88-a-candidate-for-the-most-important-const/ Change-Id: I3d62c99bbd83cc290f5992077258af156eaa6330 --- M be/src/common/status.h M be/src/exec/kudu-util.h M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/util/jni-util.h 5 files changed, 23 insertions(+), 22 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/11153/4 -- To view, visit http://gerrit.cloudera.org:8080/11153 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3d62c99bbd83cc290f5992077258af156eaa6330 Gerrit-Change-Number: 11153 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7213: Port ReportExecStatus() RPC to use KRPC
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10855 ) Change subject: IMPALA-7213: Port ReportExecStatus() RPC to use KRPC .. Patch Set 8: (2 comments) http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.cc@317 PS8, Line 317: MergeErrorMaps(instance_exec_status.error_log(), &error_log_); > This is less severe, but I guess that this is already wrong too. We potenti I think if we want to solve this idempotency issue generally we would probably just want to add sequence numbers to the report, and ignore any reports with lower sequence numbers than we've already seen. I was hesitating suggesting this because in theory this patch is just trying to port rather than also fix these other issues, but if we want to go there that's the route I'd suggest http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/service/control-service.cc File be/src/service/control-service.cc: http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/service/control-service.cc@76 PS8, Line 76: > I feel that we can still keep the deserialization here and just check for t +1 -- To view, visit http://gerrit.cloudera.org:8080/10855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe Gerrit-Change-Number: 10855 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 08 Aug 2018 23:12:14 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Exclude files with long mangled symbols from whitespace checks
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/11169 ) Change subject: Exclude files with long mangled symbols from whitespace checks .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11169 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia99cfc9840cc0d80d2654089f7e776344831a6b2 Gerrit-Change-Number: 11169 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 08 Aug 2018 23:09:05 + Gerrit-HasComments: No
[Impala-ASF-CR] Exclude files with long mangled symbols from whitespace checks
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11169 ) Change subject: Exclude files with long mangled symbols from whitespace checks .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/257/ : 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/11169 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia99cfc9840cc0d80d2654089f7e776344831a6b2 Gerrit-Change-Number: 11169 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 08 Aug 2018 23:04:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/10998 ) Change subject: IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics .. Patch Set 13: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 Gerrit-Change-Number: 10998 Gerrit-PatchSet: 13 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 08 Aug 2018 23:02:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10998 ) Change subject: IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics .. Patch Set 13: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/258/ : 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/10998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 Gerrit-Change-Number: 10998 Gerrit-PatchSet: 13 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 08 Aug 2018 23:00:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6481: [DOCS] Documented WIDTH BUCKET function
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11170 ) Change subject: IMPALA-6481: [DOCS] Documented WIDTH_BUCKET function .. Patch Set 1: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/33/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/11170 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife9577a65fe342fde160c7cb5fa666e407d5b093 Gerrit-Change-Number: 11170 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Wed, 08 Aug 2018 22:55:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6481: [DOCS] Documented WIDTH BUCKET function
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11170 ) Change subject: IMPALA-6481: [DOCS] Documented WIDTH_BUCKET function .. Patch Set 1: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/33/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/11170 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife9577a65fe342fde160c7cb5fa666e407d5b093 Gerrit-Change-Number: 11170 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 08 Aug 2018 22:53:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6481: [DOCS] Documented WIDTH BUCKET function
Alex Rodoni has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11170 Change subject: IMPALA-6481: [DOCS] Documented WIDTH_BUCKET function .. IMPALA-6481: [DOCS] Documented WIDTH_BUCKET function Change-Id: Ife9577a65fe342fde160c7cb5fa666e407d5b093 --- M docs/topics/impala_math_functions.xml 1 file changed, 41 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/11170/1 -- To view, visit http://gerrit.cloudera.org:8080/11170 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ife9577a65fe342fde160c7cb5fa666e407d5b093 Gerrit-Change-Number: 11170 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni
[Impala-ASF-CR] IMPALA-7096: restore scanner thread memory heuristics
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11103 ) Change subject: IMPALA-7096: restore scanner thread memory heuristics .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/256/ : 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/11103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9907fa8c4d2b0b85f67f4f160899c1c258ad82b Gerrit-Change-Number: 11103 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 22:40:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/10998 ) Change subject: IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/10998/11/be/src/util/default-path-handlers.cc File be/src/util/default-path-handlers.cc: http://gerrit.cloudera.org:8080/#/c/10998/11/be/src/util/default-path-handlers.cc@261 PS11, Line 261: webserver->RegisterUrlCallback("/jmx", "raw_text.tmpl", JmxHandler); > It seems like we could change raw_callback() to populate a content_type and Done -- To view, visit http://gerrit.cloudera.org:8080/10998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 Gerrit-Change-Number: 10998 Gerrit-PatchSet: 11 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 08 Aug 2018 22:40:11 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics
Hello Tianyi Wang, Philip Zeyliger, Todd Lipcon, Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10998 to look at the new patch set (#13). Change subject: IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics .. IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics Pause monitor: = This commit adds a stripped down version of Hadoop's JvmPauseMonitor class (https://bit.ly/2O6qSwm) . The core implementaion is borrowed from hadoop-common project and the hadoop dependencies are removed. - Removed dependency on AbstractService. - Not relying on Hadoop's Configuration object for reading confs. - Switched to Guava's implementation of Stopwatch. This utility class can detect both GC/non-GC pauses. In case of GC pauses, the GC metrics during the pause period are logged. Sample Output: = Detected pause in JVM or host machine (eg GC): pause of approximately 2356ms GC pool 'PS MarkSweep' had collection(s): count=1 time=2241ms GC pool 'PS Scavenge' had collection(s): count=3 time=352ms Detected pause in JVM or host machine (eg GC): pause of approximately 1964ms GC pool 'PS MarkSweep' had collection(s): count=1 time=2082ms GC pool 'PS Scavenge' had collection(s): count=1 time=251ms Detected pause in JVM or host machine (eg GC): pause of approximately 2120ms GC pool 'PS MarkSweep' had collection(s): count=1 time=2454ms Detected pause in JVM or host machine (eg GC): pause of approximately 2238ms GC pool 'PS MarkSweep' had collection(s): count=5 time=13464ms Detected pause in JVM or host machine (eg GC): pause of approximately 2233ms GC pool 'PS MarkSweep' had collection(s): count=1 time=2733ms JMX Metrics: JMX metrics are now emmitted for Impala and Catalog JVMs at the web end point /jmx. - Impalad: http(s)://:25000/jmx - Catalogd: http(s)://:25020/jmx Misc: Renamed JvmMetric -> JvmMemoryMetric to make the intent more clear. It doesn't relate to the functionality of the patch in anyway. Testing: === - Tested it manually with kill -SIGSTOP/-SIGCONT . Made sure that the non-GC JVM pauses are logged. - This class' functionality is tested manually by invoking it's main() - Injected a memory leak into the Catalog server code and made sure the GC is detected. Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 --- M be/src/common/init.cc M be/src/service/impala-http-handler.cc M be/src/util/default-path-handlers.cc M be/src/util/jni-util.cc M be/src/util/jni-util.h M be/src/util/memory-metrics.cc M be/src/util/memory-metrics.h M be/src/util/metrics-test.cc M be/src/util/webserver-test.cc M be/src/util/webserver.cc M be/src/util/webserver.h M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/common/JniUtil.java A fe/src/main/java/org/apache/impala/util/JMXJsonUtil.java A fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java A fe/src/test/java/org/apache/impala/util/JMXJsonUtilTest.java A fe/src/test/java/org/apache/impala/util/JniUtilTest.java A tests/custom_cluster/test_pause_monitor.py M tests/webserver/test_web_pages.py 19 files changed, 850 insertions(+), 90 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/10998/13 -- To view, visit http://gerrit.cloudera.org:8080/10998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 Gerrit-Change-Number: 10998 Gerrit-PatchSet: 13 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] Exclude files with long mangled symbols from whitespace checks
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11169 to look at the new patch set (#2). Change subject: Exclude files with long mangled symbols from whitespace checks .. Exclude files with long mangled symbols from whitespace checks Change-Id: Ia99cfc9840cc0d80d2654089f7e776344831a6b2 --- M bin/jenkins/critique-gerrit-review.py 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/11169/2 -- To view, visit http://gerrit.cloudera.org:8080/11169 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia99cfc9840cc0d80d2654089f7e776344831a6b2 Gerrit-Change-Number: 11169 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-110: Support for multiple DISTINCT
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10771 ) Change subject: IMPALA-110: Support for multiple DISTINCT .. Patch Set 4: (1 comment) Starting another pass over the backend now http://gerrit.cloudera.org:8080/#/c/10771/1/be/src/exec/aggregation-node.cc File be/src/exec/aggregation-node.cc: http://gerrit.cloudera.org:8080/#/c/10771/1/be/src/exec/aggregation-node.cc@118 PS1, Line 118: > Unfortunately, we can't rely on the batches being strictly ordered by agg_i Thanks for doing the experiment, seems to make sense to go with this approach for now. -- To view, visit http://gerrit.cloudera.org:8080/10771 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I055402eaef6d81e5f70e850d9f8a621e766830a4 Gerrit-Change-Number: 10771 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 08 Aug 2018 22:33:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7361: Fix flakiness in test heterogeneous proc mem limit
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11166 ) Change subject: IMPALA-7361: Fix flakiness in test_heterogeneous_proc_mem_limit .. Patch Set 1: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2955/ -- To view, visit http://gerrit.cloudera.org:8080/11166 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3e5c9b0c6a99d7157640c02e6b3c808b4ae9e73c Gerrit-Change-Number: 11166 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 22:23:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7096: restore scanner thread memory heuristics
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11103 ) Change subject: IMPALA-7096: restore scanner thread memory heuristics .. Patch Set 7: Code-Review+1 carry -- To view, visit http://gerrit.cloudera.org:8080/11103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9907fa8c4d2b0b85f67f4f160899c1c258ad82b Gerrit-Change-Number: 11103 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 22:17:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7096: restore scanner thread memory heuristics
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11103 ) Change subject: IMPALA-7096: restore scanner thread memory heuristics .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2961/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9907fa8c4d2b0b85f67f4f160899c1c258ad82b Gerrit-Change-Number: 11103 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 22:17:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7096: restore scanner thread memory heuristics
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11103 ) Change subject: IMPALA-7096: restore scanner thread memory heuristics .. Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/11103/6/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/11103/6/be/src/exec/hdfs-scan-node.cc@55 PS6, Line 55: he > nit: The Done http://gerrit.cloudera.org:8080/#/c/11103/6/be/src/exec/kudu-scan-node.h File be/src/exec/kudu-scan-node.h: http://gerrit.cloudera.org:8080/#/c/11103/6/be/src/exec/kudu-scan-node.h@77 PS6, Line 77: GetEstimatedMemPerThread > nit: maybe have the same name here as in hdfs-scan node Done http://gerrit.cloudera.org:8080/#/c/11103/6/be/src/exec/kudu-scan-node.cc File be/src/exec/kudu-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/11103/6/be/src/exec/kudu-scan-node.cc@159 PS6, Line 159: / Cases 5, 6 and 7. > nit: copy-paste error Not sure how I didn't see that when reading through the patch. http://gerrit.cloudera.org:8080/#/c/11103/6/be/src/runtime/scanner-mem-limiter.h File be/src/runtime/scanner-mem-limiter.h: http://gerrit.cloudera.org:8080/#/c/11103/6/be/src/runtime/scanner-mem-limiter.h@30 PS6, Line 30: /// Class to keep track of the global state of scanner threads and how much memory > nit: I know its implied, but maybe just mention explicitly that it is used Good point - "global" was a bad choice http://gerrit.cloudera.org:8080/#/c/11103/6/be/src/runtime/scanner-mem-limiter.h@42 PS6, Line 42: Each 'node' can only be registered once. > maybe add a dcheck for that Done -- To view, visit http://gerrit.cloudera.org:8080/11103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9907fa8c4d2b0b85f67f4f160899c1c258ad82b Gerrit-Change-Number: 11103 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 22:17:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7096: restore scanner thread memory heuristics
Hello Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11103 to look at the new patch set (#7). Change subject: IMPALA-7096: restore scanner thread memory heuristics .. IMPALA-7096: restore scanner thread memory heuristics This restores some of the heuristics removed in IMPALA-4835 that can help scans from hitting OOM conditions. The heuristics are implemented at the query level rather than in each scan node in isolation. Introduce a ScannerMemLimiter class that belongs to the QueryState that tracks the amount of memory estimated to be consumed for all scanner threads running for the query on the current backend. Also check soft memory limits to see if scanner threads should be started or the current scanner thread should stop. The long-term plan is to switch to the MT scan node implementations. When that happens this code can be removed. In the meantime this code is imperfect but will help avoid OOM in many scenarios. Testing: Added regression tests for HDFS and Kudu where we previously could run out of memory with a low mem_limit. Manual testing: * Ran query tests with --thread_creation_fault_injection=true for a bit, confirmed no crashes. * ran single-node stress test for Kudu and Parquet for 10-20 min each. Change-Id: Ib9907fa8c4d2b0b85f67f4f160899c1c258ad82b --- M be/src/exec/hdfs-scan-node.cc M be/src/exec/hdfs-scan-node.h M be/src/exec/kudu-scan-node-base.h M be/src/exec/kudu-scan-node.cc M be/src/exec/kudu-scan-node.h M be/src/exec/scan-node.cc M be/src/exec/scan-node.h M be/src/runtime/CMakeLists.txt M be/src/runtime/query-state.cc M be/src/runtime/query-state.h A be/src/runtime/scanner-mem-limiter.cc A be/src/runtime/scanner-mem-limiter.h A testdata/workloads/functional-query/queries/QueryTest/hdfs-scanner-thread-mem-scaling.test A testdata/workloads/functional-query/queries/QueryTest/kudu-scan-mem-usage.test M tests/query_test/test_mem_usage_scaling.py 15 files changed, 446 insertions(+), 49 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/11103/7 -- To view, visit http://gerrit.cloudera.org:8080/11103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib9907fa8c4d2b0b85f67f4f160899c1c258ad82b Gerrit-Change-Number: 11103 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Switch a couple of lists to deques in AnalyticEvalNode
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11159 ) Change subject: Switch a couple of lists to deques in AnalyticEvalNode .. Switch a couple of lists to deques in AnalyticEvalNode I noticed this inefficiency while looking at this code. std::deque generally offers better performance because it does fewer memory allocations and has better memory locality. The main advantages of std::list are O(1) insert/delete in the middle of the list and stable iterators that remain valid through list modifications, but neither property is useful for these result_tuples_ and window_tuples_ because we only append at the back, remove from the front and iterate over the whole list at once. Change-Id: Iaa716dcf241a0a9e4f5221e5cf7a1596c75aecc0 Reviewed-on: http://gerrit.cloudera.org:8080/11159 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/exec/analytic-eval-node.cc M be/src/exec/analytic-eval-node.h 2 files changed, 13 insertions(+), 14 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11159 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iaa716dcf241a0a9e4f5221e5cf7a1596c75aecc0 Gerrit-Change-Number: 11159 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] Switch a couple of lists to deques in AnalyticEvalNode
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11159 ) Change subject: Switch a couple of lists to deques in AnalyticEvalNode .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11159 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa716dcf241a0a9e4f5221e5cf7a1596c75aecc0 Gerrit-Change-Number: 11159 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 08 Aug 2018 22:03:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7213: Port ReportExecStatus() RPC to use KRPC
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/10855 ) Change subject: IMPALA-7213: Port ReportExecStatus() RPC to use KRPC .. Patch Set 8: (4 comments) http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.cc@310 PS8, Line 310: coord_ I feel that allowing for a non-const pointer to the Coordinator object to the Coordinator's child objects is quite dangerous in terms of accidentally introducing bugs in future changes. I'd rather prefer if a pointer to the DmlExecState is passed to this function, and the 'coord_' is left as a const ref. http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.cc@317 PS8, Line 317: MergeErrorMaps(instance_exec_status.error_log(), &error_log_); This is less severe, but I guess that this is already wrong too. We potentially can merge the same error multiple times if there's a ReportExecStatus() RPC retry due to the coordinator's ACK being lost on the first try, and thus the executor retries and sends the same payload twice. I'm wondering if it makes sense to also send this only on 'done = true'. http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.cc@516 PS8, Line 516: lock_guard l1(exec_summary->lock); Are we violating any lock ordering here? Previously, we would obtain 'exec_summary->lock' before 'lock_', but now we're doing the opposite. I looked through the code and it doesn't seem like it, but just want to make sure. http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/service/control-service.cc File be/src/service/control-service.cc: http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/service/control-service.cc@76 PS8, Line 76: I feel that we can still keep the deserialization here and just check for the (instance_exec_status_size() == 0) case. That will keep the code simpler and remove the lock.unlock() thing inside the CoordinatorBackendState. -- To view, visit http://gerrit.cloudera.org:8080/10855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe Gerrit-Change-Number: 10855 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 08 Aug 2018 22:02:08 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7409. CatalogObjectVersionSet should not allow duplicates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11151 ) Change subject: IMPALA-7409. CatalogObjectVersionSet should not allow duplicates .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2960/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11151 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I318e59c6a7fe559e86693fce1255fe6d9f829ccf Gerrit-Change-Number: 11151 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 08 Aug 2018 21:59:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6153: Execute UpdateFilter() only for executing queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 ) Change subject: IMPALA-6153: Execute UpdateFilter() only for executing queries .. Patch Set 10: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2959/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c Gerrit-Change-Number: 11005 Gerrit-PatchSet: 10 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 21:48:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6153: Execute UpdateFilter() only for executing queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 ) Change subject: IMPALA-6153: Execute UpdateFilter() only for executing queries .. Patch Set 10: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c Gerrit-Change-Number: 11005 Gerrit-PatchSet: 10 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 21:47:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6153: Execute UpdateFilter() only for executing queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 ) Change subject: IMPALA-6153: Execute UpdateFilter() only for executing queries .. Patch Set 10: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2958/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c Gerrit-Change-Number: 11005 Gerrit-PatchSet: 10 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 21:47:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6153: Execute UpdateFilter() only for executing queries
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 ) Change subject: IMPALA-6153: Execute UpdateFilter() only for executing queries .. Patch Set 9: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c Gerrit-Change-Number: 11005 Gerrit-PatchSet: 9 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 21:47:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6034: Add scanned bytes limits per query
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11081 ) Change subject: IMPALA-6034: Add scanned bytes limits per query .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11081 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3e85f80b70b3fce47e637e9322ed0316ee84f6a9 Gerrit-Change-Number: 11081 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 08 Aug 2018 21:44:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7231: group plan nodes into pipelines
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10848 ) Change subject: IMPALA-7231: group plan nodes into pipelines .. Patch Set 2: Any more thoughts? -- To view, visit http://gerrit.cloudera.org:8080/10848 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1d10eb14d997242f445e5c5fc5362d5410370721 Gerrit-Change-Number: 10848 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 21:43:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7320. Avoid calling getFileStatus() for each partition when table is loaded
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11027 ) Change subject: IMPALA-7320. Avoid calling getFileStatus() for each partition when table is loaded .. Patch Set 8: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2957/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11027 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83e5ebc214d6620d165e13f8cc80f8fdda100734 Gerrit-Change-Number: 11027 Gerrit-PatchSet: 8 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 08 Aug 2018 21:39:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7386. Replace CatalogObjectVersionQueue with a multiset
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11109 ) Change subject: IMPALA-7386. Replace CatalogObjectVersionQueue with a multiset .. Patch Set 3: Code-Review+2 Forwarding +2 -- To view, visit http://gerrit.cloudera.org:8080/11109 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1b6c58a1acef9b02fcc26a04d048ee9cc47dc0ef Gerrit-Change-Number: 11109 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 08 Aug 2018 21:28:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7386. Replace CatalogObjectVersionQueue with a multiset
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11109 ) Change subject: IMPALA-7386. Replace CatalogObjectVersionQueue with a multiset .. IMPALA-7386. Replace CatalogObjectVersionQueue with a multiset The implementation of CatalogObjectVersionQueue was based on a priority queue, which is not a good choice of data structure for the use case. This class exposes the following operations, with corresponding runtimes for the heap: - addVersion: O(lg n) - removeVersion: O(n) - getMinVersion: O(1) Given that every update of a catalog object requires removing the old version, the O(n) runtime could get quite expensive, particularly for operations like INVALIDATE METADATA which end up removing the old version of all of the objects in the catalog -- thus becoming accidentally quadratic. The new patch switches to a TreeMultiset coupled with a simple cache of the min element, with runtimes: - addVersion: O(lg n) - removeVersion: O(lg n) - getMinVersion: O(1) The downside of this patch is increased memory overhead: we are going from a PriorityQueue which has 8 bytes overhead per element to a TreeMultiset which has 48 bytes overhead per element. In a catalog with millions of tables this won't be insignificant; however, in such a catalog the performance savings are probably critical. According to a quick JMH benchmark, removal of an element from a PriorityQueue with 1M elements takes about 3.5ms, so an operation which invalidates all million tables would take about 3500 seconds without this patch. Meanwhile, the same benchmark using a TreeSet takes about 28ns per removal, so the whole invalidation would take about 28ms. This patch also makes the data structure synchronized, just for simplicity's sake, since uncontended locks are very cheap in Java. Change-Id: I1b6c58a1acef9b02fcc26a04d048ee9cc47dc0ef Reviewed-on: http://gerrit.cloudera.org:8080/11109 Tested-by: Todd Lipcon Reviewed-by: Todd Lipcon --- M fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java M fe/src/main/java/org/apache/impala/catalog/CatalogObjectCache.java D fe/src/main/java/org/apache/impala/catalog/CatalogObjectVersionQueue.java A fe/src/main/java/org/apache/impala/catalog/CatalogObjectVersionSet.java M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java A fe/src/test/java/org/apache/impala/catalog/CatalogObjectVersionSetTest.java 6 files changed, 223 insertions(+), 94 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11109 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I1b6c58a1acef9b02fcc26a04d048ee9cc47dc0ef Gerrit-Change-Number: 11109 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7410: fix centos6 dependency
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11168 ) Change subject: IMPALA-7410: fix centos6 dependency .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11168 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0fd2ddac5d818a0118c74947532b773e0733 Gerrit-Change-Number: 11168 Gerrit-PatchSet: 2 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 21:24:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7410: fix centos6 dependency
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11168 ) Change subject: IMPALA-7410: fix centos6 dependency .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2956/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11168 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0fd2ddac5d818a0118c74947532b773e0733 Gerrit-Change-Number: 11168 Gerrit-PatchSet: 2 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 21:24:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7383: Configurable HMS and Sentry policy DB
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/11104 ) Change subject: IMPALA-7383: Configurable HMS and Sentry policy DB .. Patch Set 5: Code-Review+2 (1 comment) This looks good to me. When this goes in, it can break people's existing dataload, so we should send out an email to dev@ to notify people and let them know that a workaround is to set METASTORE_DB=hive_impala and SENTRY_DB=sentry_policy (which can go in bin/impala-config-local.sh or their env before sourcing bin/impala-config.sh). http://gerrit.cloudera.org:8080/#/c/11104/5/bin/impala-config.sh File bin/impala-config.sh: http://gerrit.cloudera.org:8080/#/c/11104/5/bin/impala-config.sh@323 PS5, Line 323: export METASTORE_DB=\ : ${METASTORE_DB-$(cut -c-63 <<< HMS$ESCAPED_IMPALA_HOME)} : export SENTRY_POLICY_DB=\ : ${SENTRY_POLICY_DB-$(cut -c-63 <<< SP$ESCAPED_IMPALA_HOME)} Nit: I think each of these would fit on a single line without the line break. -- To view, visit http://gerrit.cloudera.org:8080/11104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I190d657cb95dfdf73ebd05e5dd24ef2a8e3156b8 Gerrit-Change-Number: 11104 Gerrit-PatchSet: 5 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Comment-Date: Wed, 08 Aug 2018 21:10:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11156 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 2: This is a clean pick of https://gerrit.cloudera.org/#/c/11007/ -- To view, visit http://gerrit.cloudera.org:8080/11156 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5f5b8a418d4816f603a64da6287ec392dbd4603f Gerrit-Change-Number: 11156 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 21:08:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7383: Configurable HMS and Sentry policy DB
Tianyi Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/11104 ) Change subject: IMPALA-7383: Configurable HMS and Sentry policy DB .. Patch Set 5: It passes the check now https://jenkins.impala.io/job/gerrit-code-review-checks/255/ -- To view, visit http://gerrit.cloudera.org:8080/11104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I190d657cb95dfdf73ebd05e5dd24ef2a8e3156b8 Gerrit-Change-Number: 11104 Gerrit-PatchSet: 5 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Comment-Date: Wed, 08 Aug 2018 20:59:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7403: fix child batch mem mgmt in analytic
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11155 ) Change subject: IMPALA-7403: fix child batch mem mgmt in analytic .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I09eb6213d47287f2addb72f8c1304085d2d48c55 Gerrit-Change-Number: 11155 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 20:51:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7403: fix child batch mem mgmt in analytic
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11155 ) Change subject: IMPALA-7403: fix child batch mem mgmt in analytic .. IMPALA-7403: fix child batch mem mgmt in analytic The core of the fix is in ProcessChildBatches(), where we copy 'prev_input_tuple_' to 'prev_input_tuple_pool_' and reset the child batch *before* the call to child(0)->GetNext(). This solves a couple of problems: * prev_input_tuple_ may be referencing memory from the child that had the needs_deep_copy() flag set and therefore will be freed or recycled when calling child(0)->GetNext() again. * 'prev_child_batch_' may have been holding onto resources that the child had flushed, which means they need to be freed before the next GetNext() call. Also refactors the logic around child_cmp_row_ to make the variable lifetime and data flow clearer. Testing: Add regression test. The test passes both with this patch alone and with IMPALA-7333 reapplied. Change-Id: I09eb6213d47287f2addb72f8c1304085d2d48c55 Reviewed-on: http://gerrit.cloudera.org:8080/11155 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/exec/analytic-eval-node.cc M be/src/exec/analytic-eval-node.h M be/src/exprs/scalar-expr-evaluator.cc M be/src/exprs/scalar-expr-evaluator.h M testdata/workloads/functional-query/queries/QueryTest/analytic-fns-tpcds.test 5 files changed, 136 insertions(+), 94 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I09eb6213d47287f2addb72f8c1304085d2d48c55 Gerrit-Change-Number: 11155 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7400: [DOCS] Updated outdated contents from impala porting
Alex Rodoni has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11154 ) Change subject: IMPALA-7400: [DOCS] Updated outdated contents from impala_porting .. IMPALA-7400: [DOCS] Updated outdated contents from impala_porting Change-Id: I3a1edb1ec5076354df9301ce12e7618d7cf607a6 Reviewed-on: http://gerrit.cloudera.org:8080/11154 Tested-by: Impala Public Jenkins Reviewed-by: Tim Armstrong --- M docs/topics/impala_porting.xml 1 file changed, 167 insertions(+), 208 deletions(-) Approvals: Impala Public Jenkins: Verified Tim Armstrong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11154 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3a1edb1ec5076354df9301ce12e7618d7cf607a6 Gerrit-Change-Number: 11154 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7400: [DOCS] Updated outdated contents from impala porting
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11154 ) Change subject: IMPALA-7400: [DOCS] Updated outdated contents from impala_porting .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11154 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a1edb1ec5076354df9301ce12e7618d7cf607a6 Gerrit-Change-Number: 11154 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 20:23:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7410: fix centos6 dependency
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11168 ) Change subject: IMPALA-7410: fix centos6 dependency .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11168 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0fd2ddac5d818a0118c74947532b773e0733 Gerrit-Change-Number: 11168 Gerrit-PatchSet: 1 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 20:22:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7400: [DOCS] Updated outdated contents from impala porting
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11154 ) Change subject: IMPALA-7400: [DOCS] Updated outdated contents from impala_porting .. Patch Set 2: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/32/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/11154 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a1edb1ec5076354df9301ce12e7618d7cf607a6 Gerrit-Change-Number: 11154 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 20:01:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7400: [DOCS] Updated outdated contents from impala porting
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11154 ) Change subject: IMPALA-7400: [DOCS] Updated outdated contents from impala_porting .. Patch Set 2: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/32/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/11154 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a1edb1ec5076354df9301ce12e7618d7cf607a6 Gerrit-Change-Number: 11154 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 19:51:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7400: [DOCS] Updated outdated contents from impala porting
Hello Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11154 to look at the new patch set (#2). Change subject: IMPALA-7400: [DOCS] Updated outdated contents from impala_porting .. IMPALA-7400: [DOCS] Updated outdated contents from impala_porting Change-Id: I3a1edb1ec5076354df9301ce12e7618d7cf607a6 --- M docs/topics/impala_porting.xml 1 file changed, 167 insertions(+), 208 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/11154/2 -- To view, visit http://gerrit.cloudera.org:8080/11154 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3a1edb1ec5076354df9301ce12e7618d7cf607a6 Gerrit-Change-Number: 11154 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7400: [DOCS] Updated outdated contents from imapala proting
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/11154 ) Change subject: IMPALA-7400: [DOCS] Updated outdated contents from imapala_proting .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/11154/1/docs/topics/impala_porting.xml File docs/topics/impala_porting.xml: http://gerrit.cloudera.org:8080/#/c/11154/1/docs/topics/impala_porting.xml@406 PS1, Line 406:Impala supports subqueries only in the FROM > I might have missed this one. This seems very out-of-date. I wonder if it's Done http://gerrit.cloudera.org:8080/#/c/11154/1/docs/topics/impala_porting.xml@448 PS1, Line 448:Impala does not support certain rarely used join types that are > I looked at postgres docs and a couple of other DBMS systems and I think th Done -- To view, visit http://gerrit.cloudera.org:8080/11154 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a1edb1ec5076354df9301ce12e7618d7cf607a6 Gerrit-Change-Number: 11154 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 19:49:14 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Exclude BuiltinsDb.java from whitespace checks
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11169 ) Change subject: Exclude BuiltinsDb.java from whitespace checks .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/254/ : 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/11169 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia99cfc9840cc0d80d2654089f7e776344831a6b2 Gerrit-Change-Number: 11169 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 08 Aug 2018 19:47:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7410: fix centos6 dependency
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11168 ) Change subject: IMPALA-7410: fix centos6 dependency .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/252/ : 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/11168 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0fd2ddac5d818a0118c74947532b773e0733 Gerrit-Change-Number: 11168 Gerrit-PatchSet: 1 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 08 Aug 2018 19:31:09 + Gerrit-HasComments: No
[Impala-ASF-CR] Switch a couple of lists to deques in AnalyticEvalNode
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11159 ) Change subject: Switch a couple of lists to deques in AnalyticEvalNode .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/251/ : 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/11159 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa716dcf241a0a9e4f5221e5cf7a1596c75aecc0 Gerrit-Change-Number: 11159 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 08 Aug 2018 19:14:32 + Gerrit-HasComments: No
[Impala-ASF-CR] Exclude BuiltinsDb.java from whitespace checks
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11169 Change subject: Exclude BuiltinsDb.java from whitespace checks .. Exclude BuiltinsDb.java from whitespace checks It has many long strings containing function symbols Change-Id: Ia99cfc9840cc0d80d2654089f7e776344831a6b2 --- M bin/jenkins/critique-gerrit-review.py 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/11169/1 -- To view, visit http://gerrit.cloudera.org:8080/11169 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ia99cfc9840cc0d80d2654089f7e776344831a6b2 Gerrit-Change-Number: 11169 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-7213: Port ReportExecStatus() RPC to use KRPC
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10855 ) Change subject: IMPALA-7213: Port ReportExecStatus() RPC to use KRPC .. Patch Set 8: (12 comments) http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/exec/data-sink.cc File be/src/exec/data-sink.cc: http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/exec/data-sink.cc@49 PS8, Line 49: const char* DataSink::ROOT_PARTITION_KEY = ""; this should be const char* const -- ie it's a const pointer to const characters. As is, the pointer is non-const and could be reassigned http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/exec/hdfs-table-writer.h File be/src/exec/hdfs-table-writer.h: http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/exec/hdfs-table-writer.h@91 PS8, Line 91: const InsertStatsPB& stats() { return stats_; } probably should be a const function as well? http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.h File be/src/runtime/coordinator-backend-state.h: http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.h@211 PS8, Line 211: Coordinator* coord_; /// Coordinator object that owns this BackendState this went from const to non-const. does this mutate the coordinator that owns it? http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.cc@50 PS8, Line 50: : coord_(coord), nit: you could just do coord_(DHECK_NOTNULL(coord)) here since DCHECK_NOTNULL passes through its parameter http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/coordinator-backend-state.cc@262 PS8, Line 262: lock.unlock(); I'm nervous about this optimization. Do we really have data that deserialization is slow enough to be worth it? What was the motivation to move the deserialization here instead of where it was before? It seems like, while you unlock the lock here, it's possible that the query could change state so that IsDone() is true after re-acquiring it, for example. http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/query-state.cc File be/src/runtime/query-state.cc: http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/query-state.cc@318 PS8, Line 318: bool done trying to understand the purpose of this parameter. I don't think this should be changed in this patch since it has nothing to do with krpc vs thrift, but shouldn't the 'done'-ness be able to be inferred from a combination of backend_exec_state_, and fis->current_state_? ie when a query or fragment fails, those states are transitioned before ReportExecStatus is called, and then we can just check that state to determine whether it's the last report or not? Anyway, something for later. http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/query-state.cc@331 PS8, Line 331: ThriftSerializer serializer(true); Why construct this here and pass it into ConstructReport? It seems ConstructReport is only called here, and this serializer is only used by ConstructReport, so it could just be constructed in there. http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/query-state.cc@359 PS8, Line 359: CHECK(sidecar_status.ok()) : << FromKuduStatus(sidecar_status, "Failed to add sidecar").GetDetail(); is there any chance that the profile would extend past max RPC size? I guess not since you configure the max RPC size to multiple GBs? http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/runtime/query-state.cc@373 PS8, Line 373: if (i < 2) SleepForMs(FLAGS_report_status_retry_interval_ms); worth a LOG(WARNING) here? http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/util/uid-util.h File be/src/util/uid-util.h: http://gerrit.cloudera.org:8080/#/c/10855/8/be/src/util/uid-util.h@61 PS8, Line 61: unique_id_pb->set_lo(t_unique_id.lo); worth a DCHECK that t_unique_id.__isset_.lo and hi? http://gerrit.cloudera.org:8080/#/c/10855/8/common/protobuf/control_service.proto File common/protobuf/control_service.proto: http://gerrit.cloudera.org:8080/#/c/10855/8/common/protobuf/control_service.proto@140 PS8, Line 140: This is sent only on completion of a fragment instance. can you be more explicit and say that this is sent only when 'done' is true? http://gerrit.cloudera.org:8080/#/c/10855/8/common/protobuf/control_service.proto@145 PS8, Line 145: // Map of TErrorCode to ErrorLogEntryPB; New errors that have not been reported to : // the coordinator by this fragment instance. Not idempotent. The done flag helps : // prevent applying the same update twice. : map error_log = 7; so this is only sent when 'done' is true? if not, then the done flag doesn't help avoid missing or duplicated errors, right? ie if these are sent for non-done fragments, the
[Impala-ASF-CR] IMPALA-7361: Fix flakiness in test heterogeneous proc mem limit
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11166 ) Change subject: IMPALA-7361: Fix flakiness in test_heterogeneous_proc_mem_limit .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2955/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11166 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3e5c9b0c6a99d7157640c02e6b3c808b4ae9e73c Gerrit-Change-Number: 11166 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 19:10:42 + Gerrit-HasComments: No