[Impala-ASF-CR] IMPALA-7356 (part 2 of ?): restrict number of coordinators
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11316 ) Change subject: IMPALA-7356 (part 2 of ?): restrict number of coordinators .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3081/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11316 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I43a6f095fc5274bf649833e0324815ebd266c0e2 Gerrit-Change-Number: 11316 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Comment-Date: Fri, 24 Aug 2018 19:09:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Pranay Singh has posted comments on this change. (
http://gerrit.cloudera.org:8080/10847 )
Change subject: IMPALA-6271: Impala daemon should log a message when it's being
shut down
..
Patch Set 12:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc
File be/src/common/init.cc:
http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc@297
PS10, Line 297: if (sigaction(SIGTERM, &action, nullptr) == -1) {
> > There is a value 1 displayed for old_action.sa_handler ...
I tried printing the old action sa_handler in impalad.INFO and a value of 1 was
displayed for sa_handler (old action). The value 1 corresponds to default
action for SIGTERM, I don't think there is a need for validating it because
default behavior of SIGTERM would result in killing of process without core
dump and that behavior is not altered.
http://gerrit.cloudera.org:8080/#/c/10847/12/be/src/common/init.cc
File be/src/common/init.cc:
http://gerrit.cloudera.org:8080/#/c/10847/12/be/src/common/init.cc@298
PS12, Line 298: "
> Ping?
Added GetStrErrMsg()
--
To view, visit http://gerrit.cloudera.org:8080/10847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29
Gerrit-Change-Number: 10847
Gerrit-PatchSet: 12
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Zoram Thanga
Gerrit-Comment-Date: Fri, 24 Aug 2018 19:20:53 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Hello Andrew Sherman, Lars Volker, Zoram Thanga, Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10847 to look at the new patch set (#13). Change subject: IMPALA-6271: Impala daemon should log a message when it's being shut down .. IMPALA-6271: Impala daemon should log a message when it's being shut down Currently Impalad does not log any message when SIGTERM is sent to impalad to terminate or to do a graceful shut down. This change logs a message when SIGTERM is received by impalad/catalogd/statestored. This logging will assist in debugging the issues seen in the field where impalad was not gracefully shut down (some other signal was generated that led to impalad/catalogd/statestored crash). Testing: --- a) Used kill to send signals to impalad/catalogd/statestored `kill -s SIGTERM ` and see the log message is being logged in impalad/catalogd/statestored.INFO. b) Ran test_breakpad.py to check that existing breakpad functionalities are not affected. c) Ran exhaustive tests without failure. d) Added new test in test_breakpad.py to handle SIGTERM for impalad/statestored/catalogd. Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 --- M be/src/catalog/catalogd-main.cc M be/src/common/init.cc M be/src/util/minidump.cc M tests/custom_cluster/test_breakpad.py 4 files changed, 60 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10847/13 -- To view, visit http://gerrit.cloudera.org:8080/10847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 Gerrit-Change-Number: 10847 Gerrit-PatchSet: 13 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11317 Change subject: IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock .. IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock The polling interval can be adjusted with --jvm_deadlock_detector_interval_s. Setting the interval to <= 0 disables the hang check. I don't think this should ever be needed but I wanted to keep it around in case we ever see false positive deadlock and need a way to disable the hang detection. After studying the OpenJDK code I concluded that the deadlock detection cost O(# java threads) and had to execute at a VM safepoint so will block progress of Java threads but not native threads. This seems cheap enough to execute at one minute intervals without impacting perfomance measurably. Testing: Modified code to replace findDeadlockedThreads() with getAllThreadIds() to induce the failure. Confirmed that it generated a core and dumped stacks to impalad.ERROR. Ran exhaustive tests, mostly to completion except for hitting IMPALA-7488. I'm open to ideas about how to write an automated test for this and/or whether it's worth it - it might be possible to add an impalad debug flag that deliberately induces a deadlock or write a backend death test, but I don't know if it's worth the cost of adding and maintaining complex test infra. Change-Id: If46f879e7ab9eeead0c6e9a09844e1356012898d --- M be/src/util/jni-util.cc M fe/src/main/java/org/apache/impala/common/JniUtil.java M fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java 3 files changed, 52 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/17/11317/1 -- To view, visit http://gerrit.cloudera.org:8080/11317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: If46f879e7ab9eeead0c6e9a09844e1356012898d Gerrit-Change-Number: 11317 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/11317 )
Change subject: IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11317/1/be/src/util/jni-util.cc
File be/src/util/jni-util.cc:
http://gerrit.cloudera.org:8080/#/c/11317/1/be/src/util/jni-util.cc@28
PS1, Line 28: DEFINE_int64(jvm_deadlock_detector_interval_s, 60, "Interval
between JVM deadlock checks. "
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/11317/1/be/src/util/jni-util.cc@219
PS1, Line 219: JniMethodDescriptor init_jvm_pm_desc = {"initPauseMonitor",
"(J)V", &init_jvm_pm_method};
line too long (91 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/11317
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If46f879e7ab9eeead0c6e9a09844e1356012898d
Gerrit-Change-Number: 11317
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Fri, 24 Aug 2018 19:28:38 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
Hello Bharath Vissapragada, Philip Zeyliger, Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11317 to look at the new patch set (#2). Change subject: IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock .. IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock The polling interval can be adjusted with --jvm_deadlock_detector_interval_s. Setting the interval to <= 0 disables the hang check. I don't think this should ever be needed but I wanted to keep it around in case we ever see false positive deadlock and need a way to disable the hang detection. After studying the OpenJDK code I concluded that the deadlock detection cost O(# java threads) and had to execute at a VM safepoint so will block progress of Java threads but not native threads. This seems cheap enough to execute at one minute intervals without impacting perfomance measurably. Testing: Modified code to replace findDeadlockedThreads() with getAllThreadIds() to induce the failure. Confirmed that it generated a core and dumped stacks to impalad.ERROR. Ran exhaustive tests, mostly to completion except for hitting IMPALA-7488. I'm open to ideas about how to write an automated test for this and/or whether it's worth it - it might be possible to add an impalad debug flag that deliberately induces a deadlock or write a backend death test, but I don't know if it's worth the cost of adding and maintaining complex test infra. Change-Id: If46f879e7ab9eeead0c6e9a09844e1356012898d --- M be/src/util/jni-util.cc M fe/src/main/java/org/apache/impala/common/JniUtil.java M fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java 3 files changed, 54 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/17/11317/2 -- To view, visit http://gerrit.cloudera.org:8080/11317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If46f879e7ab9eeead0c6e9a09844e1356012898d Gerrit-Change-Number: 11317 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/11317 )
Change subject: IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11317/1/be/src/util/jni-util.cc
File be/src/util/jni-util.cc:
http://gerrit.cloudera.org:8080/#/c/11317/1/be/src/util/jni-util.cc@28
PS1, Line 28: DEFINE_int64(jvm_deadlock_detector_interval_s, 60, "Interval
between JVM deadlock checks. "
> line too long (91 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/11317/1/be/src/util/jni-util.cc@219
PS1, Line 219: JniMethodDescriptor init_jvm_pm_desc = {"initPauseMonitor",
"(J)V", &init_jvm_pm_method};
> line too long (91 > 90)
Done
--
To view, visit http://gerrit.cloudera.org:8080/11317
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If46f879e7ab9eeead0c6e9a09844e1356012898d
Gerrit-Change-Number: 11317
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Fri, 24 Aug 2018 19:30:02 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7356 (part 2 of ?): restrict number of coordinators
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11316 ) Change subject: IMPALA-7356 (part 2 of ?): restrict number of coordinators .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/473/ : 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/11316 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I43a6f095fc5274bf649833e0324815ebd266c0e2 Gerrit-Change-Number: 11316 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Comment-Date: Fri, 24 Aug 2018 19:32:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10847 ) Change subject: IMPALA-6271: Impala daemon should log a message when it's being shut down .. Patch Set 13: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/474/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/10847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 Gerrit-Change-Number: 10847 Gerrit-PatchSet: 13 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Fri, 24 Aug 2018 19:54:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11317 ) Change subject: IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/475/ : 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/11317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If46f879e7ab9eeead0c6e9a09844e1356012898d Gerrit-Change-Number: 11317 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 24 Aug 2018 20:02:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11317 ) Change subject: IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/476/ : 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/11317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If46f879e7ab9eeead0c6e9a09844e1356012898d Gerrit-Change-Number: 11317 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 24 Aug 2018 20:10:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Hbase Scan Nodes
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11306 ) Change subject: IMPALA-7351: Improve memory estimates for Hbase Scan Nodes .. Patch Set 1: (1 comment) Did another pass - my main concerns are testing (which you're working on) and the understandability of the estimation method (which I mentioned earlier). http://gerrit.cloudera.org:8080/#/c/11306/1/be/src/exec/hbase-table-scanner.cc File be/src/exec/hbase-table-scanner.cc: http://gerrit.cloudera.org:8080/#/c/11306/1/be/src/exec/hbase-table-scanner.cc@107 PS1, Line 107: value_pool_(new MemPool(scan_node_->mem_tracker(),true)), whitespace -- To view, visit http://gerrit.cloudera.org:8080/11306 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I583545c3f5e454854f111871c5fbc4f108ae4bff Gerrit-Change-Number: 11306 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 24 Aug 2018 20:36:39 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7421. Static methods use wrong JNI call function
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11181 ) Change subject: IMPALA-7421. Static methods use wrong JNI call function .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11181 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If7cde6ca91613b63afe5307f4d819fb24cb17fd6 Gerrit-Change-Number: 11181 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 24 Aug 2018 20:42:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7421. Static methods use wrong JNI call function
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11181 ) Change subject: IMPALA-7421. Static methods use wrong JNI call function .. Patch Set 3: Looks good modulo Bharath's comments and assuming other reviewers are ok. Noticed this conflicts with my patch so would be nice to get it in so I can rebase onto it. -- To view, visit http://gerrit.cloudera.org:8080/11181 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If7cde6ca91613b63afe5307f4d819fb24cb17fd6 Gerrit-Change-Number: 11181 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 24 Aug 2018 20:43:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/11317 )
Change subject: IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
..
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/11317/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11317/2//COMMIT_MSG@22
PS2, Line 22: findDeadlockedThreads
Maybe worth injecting a deadlock locally and see if findDeadlockedThreads()
works as expected too (if you have not done that already).
http://gerrit.cloudera.org:8080/#/c/11317/2/be/src/util/jni-util.cc
File be/src/util/jni-util.cc:
http://gerrit.cloudera.org:8080/#/c/11317/2/be/src/util/jni-util.cc@28
PS2, Line 28: 60
IMO, configuring it something like 300s is reasonable too since VM seems to be
executing a safepoint for this. Also since we see deadlocks very rarely.
http://gerrit.cloudera.org:8080/#/c/11317/2/be/src/util/jni-util.cc@29
PS2, Line 29: "Interval between JVM deadlock checks. If set to 0 or a
negative value, deadlock "
nit: mark it (Advanced)?
http://gerrit.cloudera.org:8080/#/c/11317/2/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java
File fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java:
http://gerrit.cloudera.org:8080/#/c/11317/2/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java@176
PS2, Line 176: return;
not related to your change but I just noticed this, can we log this as an error
with exception trace so that we know if the pause monitor exits.
http://gerrit.cloudera.org:8080/#/c/11317/2/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java@217
PS2, Line 217: LOG.fatal("Aborting because of deadlocked threads in
JVM.");
Should we dump the entire thread stacks (not just the deadlocked ones) and
other JVM state, say JMX output at this point? (gives heap usage etc. if
needed). Also any other stuff like /metrics etc?
--
To view, visit http://gerrit.cloudera.org:8080/11317
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If46f879e7ab9eeead0c6e9a09844e1356012898d
Gerrit-Change-Number: 11317
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Fri, 24 Aug 2018 20:47:08 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11317 ) Change subject: IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11317/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11317/2//COMMIT_MSG@17 PS2, Line 17: cheap Just curious if you tried this out on a loaded JVM with a bunch of threads etc and how long it takes.. -- To view, visit http://gerrit.cloudera.org:8080/11317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If46f879e7ab9eeead0c6e9a09844e1356012898d Gerrit-Change-Number: 11317 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 24 Aug 2018 20:50:09 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6758: Add metric for current catalog version to catalog
Tianyi Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/11293 ) Change subject: IMPALA-6758: Add metric for current catalog version to catalog .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11293 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff7e158292ca9e5a17663e5bfc74931cc57c0328 Gerrit-Change-Number: 11293 Gerrit-PatchSet: 3 Gerrit-Owner: Vincent Tran Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vincent Tran Gerrit-Comment-Date: Fri, 24 Aug 2018 20:58:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7421. Static methods use wrong JNI call function
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11181 ) Change subject: IMPALA-7421. Static methods use wrong JNI call function .. Patch Set 3: yep, sorry for the delay, will finish this up this afternoon. -- To view, visit http://gerrit.cloudera.org:8080/11181 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If7cde6ca91613b63afe5307f4d819fb24cb17fd6 Gerrit-Change-Number: 11181 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 24 Aug 2018 22:19:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7356 (part 2 of ?): restrict number of coordinators
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11316 ) Change subject: IMPALA-7356 (part 2 of ?): restrict number of coordinators .. IMPALA-7356 (part 2 of ?): restrict number of coordinators The immediate motivation is to allow testing admission control with a single coordinator where distributed overadmission is not possible. Testing: Ran locally against a 3 node minicluster with max-coordinators unset and set to 1 and 3. Confirmed that queries were submitted to the expected impalads in all cases. Change-Id: I43a6f095fc5274bf649833e0324815ebd266c0e2 Reviewed-on: http://gerrit.cloudera.org:8080/11316 Reviewed-by: Michael Brown Tested-by: Impala Public Jenkins --- M tests/stress/concurrent_select.py 1 file changed, 10 insertions(+), 1 deletion(-) Approvals: Michael Brown: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11316 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I43a6f095fc5274bf649833e0324815ebd266c0e2 Gerrit-Change-Number: 11316 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown
[Impala-ASF-CR] [DOCS] Known Issues for 3.1
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11323 ) Change subject: [DOCS] Known Issues for 3.1 .. Patch Set 1: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/53/ 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/11323 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I247438f28835c1986deca39e98cd7deb4dc20351 Gerrit-Change-Number: 11323 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 24 Aug 2018 22:26:41 + Gerrit-HasComments: No
[Impala-ASF-CR] [DOCS] Known Issues for 3.1
Alex Rodoni has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11323 Change subject: [DOCS] Known Issues for 3.1 .. [DOCS] Known Issues for 3.1 Change-Id: I247438f28835c1986deca39e98cd7deb4dc20351 --- M docs/topics/impala_known_issues.xml 1 file changed, 0 insertions(+), 166 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/11323/1 -- To view, visit http://gerrit.cloudera.org:8080/11323 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I247438f28835c1986deca39e98cd7deb4dc20351 Gerrit-Change-Number: 11323 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni
[Impala-ASF-CR] IMPALA-7356 (part 2 of ?): restrict number of coordinators
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11316 ) Change subject: IMPALA-7356 (part 2 of ?): restrict number of coordinators .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11316 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I43a6f095fc5274bf649833e0324815ebd266c0e2 Gerrit-Change-Number: 11316 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Comment-Date: Fri, 24 Aug 2018 22:26:24 + Gerrit-HasComments: No
[Impala-ASF-CR] [DOCS] Known Issues for 3.1 (WIP)
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11323 to look at the new patch set (#2). Change subject: [DOCS] Known Issues for 3.1 (WIP) .. [DOCS] Known Issues for 3.1 (WIP) Change-Id: I247438f28835c1986deca39e98cd7deb4dc20351 --- M docs/topics/impala_known_issues.xml 1 file changed, 0 insertions(+), 166 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/11323/2 -- To view, visit http://gerrit.cloudera.org:8080/11323 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I247438f28835c1986deca39e98cd7deb4dc20351 Gerrit-Change-Number: 11323 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7421. Static methods use wrong JNI call function
Hello Bharath Vissapragada, Philip Zeyliger, Csaba Ringhofer, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11181 to look at the new patch set (#4). Change subject: IMPALA-7421. Static methods use wrong JNI call function .. IMPALA-7421. Static methods use wrong JNI call function Various places throughout the code were using the JNI CallObjectMethod function when in fact they should have been using CallStaticObjectMethod. It appears that this doesn't cause crashes in release builds, but if running with -Xcheck:jni, it causes an assertion failure: guarantee(method->size_of_parameters() == size_of_parameters()) failed: wrong no. of arguments pushed This patch cleans up JniUtil a bit to add a "fluent" style utility for calling JNI functions. This calling style should make the above mistake more obvious because it forces the caller to write 'on_class(...)' for static methods and 'on_instance(...)' for instance methods. Change-Id: If7cde6ca91613b63afe5307f4d819fb24cb17fd6 --- M be/src/common/init.cc M be/src/common/status.h M be/src/service/fe-support.h M be/src/service/frontend.cc M be/src/util/backend-gflag-util.h M be/src/util/jni-util.cc M be/src/util/jni-util.h M be/src/util/logging-support.cc M be/src/util/logging-support.h M be/src/util/zip-util.cc M be/src/util/zip-util.h 11 files changed, 184 insertions(+), 94 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/11181/4 -- To view, visit http://gerrit.cloudera.org:8080/11181 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If7cde6ca91613b63afe5307f4d819fb24cb17fd6 Gerrit-Change-Number: 11181 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7421. Static methods use wrong JNI call function
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11181 ) Change subject: IMPALA-7421. Static methods use wrong JNI call function .. Patch Set 3: Code-Review+2 (2 comments) Rebased and fixed the indentation issue. Carrying +2s. http://gerrit.cloudera.org:8080/#/c/11181/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11181/2//COMMIT_MSG@12 PS2, Line 12: -Xcheck:jni > mind filing a jira for this? Filed IMPALA-7489 http://gerrit.cloudera.org:8080/#/c/11181/3/be/src/util/jni-util.h File be/src/util/jni-util.h: http://gerrit.cloudera.org:8080/#/c/11181/3/be/src/util/jni-util.h@126 PS3, Line 126: > nit: indentation off. Done -- To view, visit http://gerrit.cloudera.org:8080/11181 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If7cde6ca91613b63afe5307f4d819fb24cb17fd6 Gerrit-Change-Number: 11181 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 24 Aug 2018 22:29:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [DOCS] Known Issues for 3.1 (WIP)
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11323 ) Change subject: [DOCS] Known Issues for 3.1 (WIP) .. Patch Set 1: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/53/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/11323 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I247438f28835c1986deca39e98cd7deb4dc20351 Gerrit-Change-Number: 11323 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 24 Aug 2018 22:29:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7421. Static methods use wrong JNI call function
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11181 ) Change subject: IMPALA-7421. Static methods use wrong JNI call function .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11181 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If7cde6ca91613b63afe5307f4d819fb24cb17fd6 Gerrit-Change-Number: 11181 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 24 Aug 2018 22:29:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7421. Static methods use wrong JNI call function
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11181 ) Change subject: IMPALA-7421. Static methods use wrong JNI call function .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3083/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11181 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If7cde6ca91613b63afe5307f4d819fb24cb17fd6 Gerrit-Change-Number: 11181 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 24 Aug 2018 22:29:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Pooja Nilangekar has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/11052 ) Change subject: IMPALA-6644: Add last heartbeat timestamp into Statestore metric .. IMPALA-6644: Add last heartbeat timestamp into Statestore metric After this patch, the statestore keeps track of the time since the last heartbeat for each subscriber. It is exposed as a subscriber metric on the statestore debug page. It also adds a monitoring thread that periodically checks the last heartbeat timestamp for all subscribers and logs the IDs of those that have not been updated since the last periodic check. Testing: Added an end to end test to verify the 'sec_since_heartbeat' metric of a slow subscriber. Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 --- M be/src/statestore/statestore.cc M be/src/statestore/statestore.h M tests/statestore/test_statestore.py M www/statestore_subscribers.tmpl 4 files changed, 107 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/11052/9 -- To view, visit http://gerrit.cloudera.org:8080/11052 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Gerrit-Change-Number: 11052 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: Todd Lipcon
[Impala-ASF-CR] IMPALA-6758: Add metric for current catalog version to catalog
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11293 ) Change subject: IMPALA-6758: Add metric for current catalog version to catalog .. Patch Set 3: (1 comment) I'm curious: what's the motivation for exposing this as a metric? Why would the operator want to monitor it? http://gerrit.cloudera.org:8080/#/c/11293/3/be/src/catalog/catalog-server.cc File be/src/catalog/catalog-server.cc: http://gerrit.cloudera.org:8080/#/c/11293/3/be/src/catalog/catalog-server.cc@185 PS3, Line 185: current_catalog_version_metric_ = catalog_metrics->AddGauge : (CATALOG_CURRENT_TOPIC_VERSION can you clarify which version this is? Confusingly there are two versions floating around the catalog code -- there is the *topic* version (set by the statestore) and the catalog version (set by the catalogd). We often use the wrong term to refer to the thing we're dealing with, and it seems we are continuing to proliferate such inclarities here. -- To view, visit http://gerrit.cloudera.org:8080/11293 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff7e158292ca9e5a17663e5bfc74931cc57c0328 Gerrit-Change-Number: 11293 Gerrit-PatchSet: 3 Gerrit-Owner: Vincent Tran Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vincent Tran Gerrit-Comment-Date: Fri, 24 Aug 2018 22:32:34 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [DOCS] Known Issues for 3.1 (WIP)
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/11323 ) Change subject: [DOCS] Known Issues for 3.1 (WIP) .. Patch Set 2: Code-Review+2 WIP. Will be further updated. -- To view, visit http://gerrit.cloudera.org:8080/11323 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I247438f28835c1986deca39e98cd7deb4dc20351 Gerrit-Change-Number: 11323 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 24 Aug 2018 22:33:06 + Gerrit-HasComments: No
[Impala-ASF-CR] [DOCS] Known Issues for 3.1 (WIP)
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/11323 ) Change subject: [DOCS] Known Issues for 3.1 (WIP) .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11323 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I247438f28835c1986deca39e98cd7deb4dc20351 Gerrit-Change-Number: 11323 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 24 Aug 2018 22:33:18 + Gerrit-HasComments: No
[Impala-ASF-CR] [DOCS] Known Issues for 3.1 (WIP)
Alex Rodoni has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11323 ) Change subject: [DOCS] Known Issues for 3.1 (WIP) .. [DOCS] Known Issues for 3.1 (WIP) Change-Id: I247438f28835c1986deca39e98cd7deb4dc20351 Reviewed-on: http://gerrit.cloudera.org:8080/11323 Reviewed-by: Alex Rodoni Tested-by: Alex Rodoni --- M docs/topics/impala_known_issues.xml 1 file changed, 0 insertions(+), 166 deletions(-) Approvals: Alex Rodoni: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11323 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I247438f28835c1986deca39e98cd7deb4dc20351 Gerrit-Change-Number: 11323 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11052 ) Change subject: IMPALA-6644: Add last heartbeat timestamp into Statestore metric .. Patch Set 9: (3 comments) http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.h File be/src/statestore/statestore.h: http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.h@447 PS8, Line 447: Topics priority_subscribed_topics_; > To incorporate the changes I changes the last_heartbeat_ts_ to kudu::MonoTi Does the worker thread which calls RefreshHeartbeatTimestamp also hold subscriber_lock_ while doing so? If not, I think it's technically required to make it atomic (or ensure that the caller does hold that lock). That's the case even though the underlying variable here happens to be int64 and therefore atomic "naturally" on x86. Accessing it without synchronization ends up in murky territory around what optimizations compilers are allowed to do, etc, and also means that if we ever implement TSAN support for running tests we'd see a bunch of warnings about incorrect synchronization. http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.cc File be/src/statestore/statestore.cc: http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.cc@389 PS8, Line 389: > Yes, it does occur (especially during initialization). I changed it from mi Even though the resolution stored is in nanoseconds it's possible that the clock source doesn't have the same resolution (ie it may step with some "coarseness" rather than being accurate to the nano). I think the particular clock implementation we use on Linux are fine, but baking that kind of timing assumption in here seems like we might just get a really surprising failure at some point down the road. I'd also think about this from the other direction: let's say we did accidentally update it twice at the same microsecond. Would it be a problem? Since we're just using this for reporting freshness of heartbeats, and not relying on it being increasing-only or something, I don't think it's worth being strict. http://gerrit.cloudera.org:8080/#/c/11052/9/be/src/statestore/statestore.cc File be/src/statestore/statestore.cc: http://gerrit.cloudera.org:8080/#/c/11052/9/be/src/statestore/statestore.cc@451 PS9, Line 451: initialized_ = true; shouldn't this actually go at the end of the function? ie if the server fails to start and returns a bad Status here, it's acceptable to destruct the Statestore - it's only once we've started the new thread that it enters the "unstoppable" state. I think with it in this position, you might trigger the CHECK failure if the SSL configuration were incorrect or the port was already bound, whereas maybe we'd want to edit in a more graceful fashion. -- To view, visit http://gerrit.cloudera.org:8080/11052 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Gerrit-Change-Number: 11052 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: Todd Lipcon Gerrit-Comment-Date: Fri, 24 Aug 2018 22:40:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/11317 )
Change subject: IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
..
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11317/2/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java
File fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java:
http://gerrit.cloudera.org:8080/#/c/11317/2/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java@216
PS2, Line 216: for (ThreadInfo thread : deadlockedThreads)
LOG.error(thread.toString());
nit: in theory, given the threads are deadlocked, it should be guaranteed that
the threads still exist when you call getThreadInfo() above. But, just in case,
I think we should be extra defensive here and check 'thread' against null
before calling thread.toString() here.
http://gerrit.cloudera.org:8080/#/c/11317/2/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java@217
PS2, Line 217: LOG.fatal("Aborting because of deadlocked threads in
JVM.");
> Should we dump the entire thread stacks (not just the deadlocked ones) and
+1 for other thread stacks.
--
To view, visit http://gerrit.cloudera.org:8080/11317
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If46f879e7ab9eeead0c6e9a09844e1356012898d
Gerrit-Change-Number: 11317
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Fri, 24 Aug 2018 22:48:36 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/11317 )
Change subject: IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11317/2/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java
File fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java:
http://gerrit.cloudera.org:8080/#/c/11317/2/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java@217
PS2, Line 217: LOG.fatal("Aborting because of deadlocked threads in
JVM.");
> +1 for other thread stacks.
oh, also, does log4j "fatal" actually guarantee the process exits? You dont
need System.exit(1) here or anything too?
--
To view, visit http://gerrit.cloudera.org:8080/11317
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If46f879e7ab9eeead0c6e9a09844e1356012898d
Gerrit-Change-Number: 11317
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Fri, 24 Aug 2018 22:49:16 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7421. Static methods use wrong JNI call function
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11181 ) Change subject: IMPALA-7421. Static methods use wrong JNI call function .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/477/ : 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/11181 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If7cde6ca91613b63afe5307f4d819fb24cb17fd6 Gerrit-Change-Number: 11181 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 24 Aug 2018 22:58:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7477: Batch-oriented query set construction
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11297 ) Change subject: IMPALA-7477: Batch-oriented query set construction .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/11297/4/be/src/service/hs2-util.cc File be/src/service/hs2-util.cc: http://gerrit.cloudera.org:8080/#/c/11297/4/be/src/service/hs2-util.cc@128 PS4, Line 128: ReserveSpace Just passing through here... is it possible that this reserving design will end up with an "accidentally quadratic" scenario in the case that rows are added a few at a time? I'm not sure at what level RowBatches get aggregated together into batches that get delivered to the client, but if you had a RowBatch size of 1000 rows, and some selective predicate such that each RowBatch only has one or two matches, would we end up calling AddRows() with num_rows=1 in a loop anywhere? If that's the case we may want to do something here like reserve the next larger power-of-two capacity rather than "size to fit this next batch" -- To view, visit http://gerrit.cloudera.org:8080/11297 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibc87a84c34935d0d5841c7f5528eb802527fa809 Gerrit-Change-Number: 11297 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 24 Aug 2018 22:58:39 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/11317 ) Change subject: IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11317/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11317/2//COMMIT_MSG@29 PS2, Line 29: I'm open to ideas about how to write an automated test for this and/or We could write a Java test that at least invokes the loop that checks for deadlocked threads. You could actually deadlock some threads, but I've found that calling .interrupt() on them is hopeless, so you'll have leaked them for the duration of the tests. That's maybe fine, but be sure to mark them as daemon threads. Or don't worry about it. For what it's worth, I found https://docs.oracle.com/javase/7/docs/technotes/guides/concurrency/threadPrimitiveDeprecation.html interesting. -- To view, visit http://gerrit.cloudera.org:8080/11317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If46f879e7ab9eeead0c6e9a09844e1356012898d Gerrit-Change-Number: 11317 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 24 Aug 2018 23:15:50 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11052 ) Change subject: IMPALA-6644: Add last heartbeat timestamp into Statestore metric .. Patch Set 9: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/478/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/11052 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Gerrit-Change-Number: 11052 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: Todd Lipcon Gerrit-Comment-Date: Fri, 24 Aug 2018 23:18:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7425: Change incremental stats to pull from catalogd.
Vuk Ercegovac has posted comments on this change. (
http://gerrit.cloudera.org:8080/11193 )
Change subject: IMPALA-7425: Change incremental stats to pull from catalogd.
..
Patch Set 2:
(12 comments)
http://gerrit.cloudera.org:8080/#/c/11193/2/common/thrift/CatalogService.thrift
File common/thrift/CatalogService.thrift:
http://gerrit.cloudera.org:8080/#/c/11193/2/common/thrift/CatalogService.thrift@379
PS2, Line 379: statistcs
> typo
Done
http://gerrit.cloudera.org:8080/#/c/11193/2/common/thrift/CatalogService.thrift@382
PS2, Line 382: 1: required CatalogObjects.TTableName table_name
: 2: required list partition_ids
> it seems risky to use partition IDs without being tied to a particular vers
Changed it to detect when the table is a differing version. That makes the
approach similar to the one taken by TTableInfoSelector which also uses ids. If
we can use ids, I'd prefer it since its less verbose. Any further reason to use
the partitioning values (its name)? Or, what's the reason that you went with
ids instead of names for TTableInfoSelector?
Also, I put the version on the request. If the catalog detects that the
versions are out-of-sync, it throws an exception. This way, we don't go through
the work of sending back stats that will be rejected by the client.
http://gerrit.cloudera.org:8080/#/c/11193/2/common/thrift/CatalogService.thrift@387
PS2, Line 387: statistcs
> typo
Done
http://gerrit.cloudera.org:8080/#/c/11193/2/common/thrift/CatalogService.thrift@388
PS2, Line 388: // If there was an error, an empty partition_stats mapping is
returned along with
> - seems like it would be easier to just make the returned map optional so y
made the return optional. clarified the comment about the returned result.
http://gerrit.cloudera.org:8080/#/c/11193/2/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
File fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java:
http://gerrit.cloudera.org:8080/#/c/11193/2/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java@336
PS2, Line 336: LOG.info("analyzing compute stats");
> probably didn't mean to leave this in at INFO level
Done
http://gerrit.cloudera.org:8080/#/c/11193/2/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java@451
PS2, Line 451: !RuntimeEnv.INSTANCE.isTestEnv()
> why not isTestEnv?
when in test mode, my understanding is that the impalad contains a catalogd, so
when trying to make an rpc to the catalog, it fails. as a result, for frontend
tests, this path is not run.
http://gerrit.cloudera.org:8080/#/c/11193/2/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java@456
PS2, Line 456: fetchedPartitionStats = fetchPartitionStats(analyzer,
partStatsToFetch);
> it seems some of this code ends up duplicated in a few places. Did you cons
I considered it but since I was only using it for the incremental case, in
order to keep things simpler, I left it out.
Yes, this method is a bit branchy already so this change didn't help that. I've
pushed both main branches down to a common method.
http://gerrit.cloudera.org:8080/#/c/11193/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/11193/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@415
PS2, Line 415: // Table is not loaded.
: if (!table.isLoaded()) {
: throw new CatalogException("Partition statistics not
available for not yet "
: + " loaded table: " + request.table_name);
> can this be the case? I thought if it's FeFsTable then it's guaranteed to b
indeed. I see the default (true) is not overridden down to that class. changed
it to a precondition.
http://gerrit.cloudera.org:8080/#/c/11193/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@428
PS2, Line 428: tableName.getDb_name() + "." + tableName.getTable_name()
> nit: fsTable.getFullName()
Done
http://gerrit.cloudera.org:8080/#/c/11193/2/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java:
http://gerrit.cloudera.org:8080/#/c/11193/2/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@524
PS2, Line 524: may not be serialized and sent from catalogd to impalads.
> I find this usage of the word "may" unclear -- not sure if you mean "it's n
re-worded
http://gerrit.cloudera.org:8080/#/c/11193/2/tests/conftest.py
File tests/conftest.py:
http://gerrit.cloudera.org:8080/#/c/11193/2/tests/conftest.py@125
PS2, Line 125: "
> flake8: E501 line too long (91 > 90 characters)
Done
http://gerrit.cloudera.org:8080/#/c/11193/2/tests/custom_cluster/test_pull_stats.py
File tests/custom_cluster/test_pull_stats.py:
http://gerrit.cloudera.org:8080/#/c/11193/2/
[Impala-ASF-CR] IMPALA-7425: Change incremental stats to pull from catalogd.
Hello Bharath Vissapragada, Tianyi Wang, Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11193 to look at the new patch set (#3). Change subject: IMPALA-7425: Change incremental stats to pull from catalogd. .. IMPALA-7425: Change incremental stats to pull from catalogd. Currently, incremental stats can consume a substantial amount of metadata memory (per table, partition, column). This metadata is transmitted from catalogd to all coordinators. As a result, memory is used for all loaded tables that use incremental stats all the time at all coordinators. A consequence is that coordinators and catalogd die from OOM more often when incremental stats are used and more network bandwidth is used. This change removes incremental stats from impalads. These stats are only needed when computing incremental statistics and merging new results with the existing results. They are not used by queries. As a result, the change requires that coordinators fetch incremental stats directly from catalogd when computing incremental stats. In addition, catalogd no longer sends incremental stats to coordinators via the statestore. The option is enabled by setting a new flag, --pull_incremental_statistics, on the catalogd and all impalad coordinators. Testing: - manual testing - added end-to-end tests with --pull_incremental_statistics enabled for the compute-stats-incremental.test - added fe CatalogTest for new catalogd service method - passes exhaustive tests when --pull_incremental_statistics is enabled and disabled Change-Id: I9d564808ca5157afe4e091909ca6cdac76e60d6e --- M be/src/catalog/catalog-server.cc M be/src/catalog/catalog-service-client-wrapper.h M be/src/catalog/catalog.cc M be/src/catalog/catalog.h M be/src/common/global-flags.cc M be/src/exec/catalog-op-executor.cc M be/src/exec/catalog-op-executor.h M be/src/service/fe-support.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M common/thrift/CatalogService.thrift M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/FeCatalog.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java M tests/common/custom_cluster_test_suite.py M tests/conftest.py A tests/custom_cluster/test_pull_stats.py 26 files changed, 593 insertions(+), 92 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/11193/3 -- To view, visit http://gerrit.cloudera.org:8080/11193 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9d564808ca5157afe4e091909ca6cdac76e60d6e Gerrit-Change-Number: 11193 Gerrit-PatchSet: 3 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7425: Change incremental stats to pull from catalogd.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11193 ) Change subject: IMPALA-7425: Change incremental stats to pull from catalogd. .. Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/11193/3/tests/custom_cluster/test_pull_stats.py File tests/custom_cluster/test_pull_stats.py: http://gerrit.cloudera.org:8080/#/c/11193/3/tests/custom_cluster/test_pull_stats.py@21 PS3, Line 21: class TestPullStatistics(CustomClusterTestSuite): flake8: E302 expected 2 blank lines, found 1 http://gerrit.cloudera.org:8080/#/c/11193/3/tests/custom_cluster/test_pull_stats.py@25 PS3, Line 25: flake8: W293 blank line contains whitespace http://gerrit.cloudera.org:8080/#/c/11193/3/tests/custom_cluster/test_pull_stats.py@25 PS3, Line 25: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/11193/3/tests/custom_cluster/test_pull_stats.py@26 PS3, Line 26: @ flake8: E303 too many blank lines (2) -- To view, visit http://gerrit.cloudera.org:8080/11193 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9d564808ca5157afe4e091909ca6cdac76e60d6e Gerrit-Change-Number: 11193 Gerrit-PatchSet: 3 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 24 Aug 2018 23:24:09 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7425: Change incremental stats to pull from catalogd.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11193 ) Change subject: IMPALA-7425: Change incremental stats to pull from catalogd. .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/479/ : 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/11193 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9d564808ca5157afe4e091909ca6cdac76e60d6e Gerrit-Change-Number: 11193 Gerrit-PatchSet: 3 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 24 Aug 2018 23:58:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
Hello Bharath Vissapragada, Philip Zeyliger, Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11317 to look at the new patch set (#3). Change subject: IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock .. IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock The polling interval can be adjusted with --jvm_deadlock_detector_interval_s. Setting the interval to <= 0 disables the hang check. I don't think this should ever be needed but I wanted to keep it around in case we ever see false positive deadlock and need a way to disable the hang detection. After studying the OpenJDK code I concluded that the deadlock detection cost O(# java threads) and had to execute at a VM safepoint so will block progress of Java threads but not native threads. This seems cheap enough to execute at one minute intervals without impacting perfomance measurably. Testing: Modified code to replace findDeadlockedThreads() with getAllThreadIds() to induce the failure in impalad and catalogd. Confirmed that it generated a core and dumped stacks to impalad.ERROR. Added manual testing in JvmPauseMonitor.main() that induces a deadlock: CLASSPATH=$CLASSPATH:fe/target/impala-frontend-0.1-SNAPSHOT.jar \ java org.apache.impala.util.JvmPauseMonitor deadlock Ran exhaustive tests, mostly to completion except for hitting IMPALA-7488. I'm open to ideas about how to write an automated test for this and/or whether it's worth it - it might be possible to add an impalad debug flag that deliberately induces a deadlock or write a backend death test, but I don't know if it's worth the cost of adding and maintaining complex test infra. Change-Id: If46f879e7ab9eeead0c6e9a09844e1356012898d --- M be/src/util/jni-util.cc M fe/src/main/java/org/apache/impala/common/JniUtil.java M fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java 3 files changed, 114 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/17/11317/3 -- To view, visit http://gerrit.cloudera.org:8080/11317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If46f879e7ab9eeead0c6e9a09844e1356012898d Gerrit-Change-Number: 11317 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/11317 )
Change subject: IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
..
Patch Set 2:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/11317/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11317/2//COMMIT_MSG@17
PS2, Line 17: cheap
> Just curious if you tried this out on a loaded JVM with a bunch of threads
I tried running a local stress test with up to 500 concurrent queries. I added
a timer around findDeadlockedThreads() 10/15 times it took 100s of microseconds
and 5/15 it took 15-50ms. I'm not sure how valid the measurement is because the
time could include the thread being descheduled by the OS and the thread
getting stuck in a GC pause.
So then I enable JVM debug output with
LIBHDFS_OPTS="${LIBHDFS_OPTS} -XX:+PrintGCApplicationStoppedTime
-XX:+PrintSafepointStatistics -XX:PrintSafepointStatisticsCount=1"
The time spent in the operation was always < 1ms but sometimes it blocked for
longer waiting to get into the safepoint. I suspect that's something to do with
the sheer number of impalad threads runnning. It looks like there's a periodic
safepoint anyway in hotspot every 1s by default so running
findDeadlockedThreads() every 60s should only make any problem 1/60 worse.
http://gerrit.cloudera.org:8080/#/c/11317/2//COMMIT_MSG@22
PS2, Line 22: findDeadlockedThreads
> Maybe worth injecting a deadlock locally and see if findDeadlockedThreads()
Extended the testing utility code to induce a deadlock.
http://gerrit.cloudera.org:8080/#/c/11317/2//COMMIT_MSG@29
PS2, Line 29: I'm open to ideas about how to write an automated test for this
and/or
> We could write a Java test that at least invokes the loop that checks for d
Yeah I once did an undergrad project where a teammate made heavy use of
thread.stop() - it wasn't the most stable bit of code...
I think I'll skip automating the test if thats ok. I think we'd want an
end-to-end test if anything - a unit test would probably mostly be checking
whether findDeadlockThreads() works. We could do a python test that launches a
program with a deadlock and checks that it exits, but it feels like overkill.
http://gerrit.cloudera.org:8080/#/c/11317/2/be/src/util/jni-util.cc
File be/src/util/jni-util.cc:
http://gerrit.cloudera.org:8080/#/c/11317/2/be/src/util/jni-util.cc@28
PS2, Line 28: 60
> IMO, configuring it something like 300s is reasonable too since VM seems to
My reason for keeping it lower was to reduce the potential duration of an
outage resulting from this - if it's 60s then I'd expect the cluster to recover
from this in < 2min (up to 60s to detect the hang, then maybe 30s for the
statestore to detect the process exit and propagate the failure).
Qualitatively a 5 minute+ outage feels worse to me than a shorter outage. As
you said though, this is rare so maybe that is ok so long as it resolves itself.
http://gerrit.cloudera.org:8080/#/c/11317/2/be/src/util/jni-util.cc@29
PS2, Line 29: "Interval between JVM deadlock checks. If set to 0 or a
negative value, deadlock "
> nit: mark it (Advanced)?
Done
http://gerrit.cloudera.org:8080/#/c/11317/2/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java
File fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java:
http://gerrit.cloudera.org:8080/#/c/11317/2/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java@176
PS2, Line 176: return;
> not related to your change but I just noticed this, can we log this as an e
Done
http://gerrit.cloudera.org:8080/#/c/11317/2/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java@216
PS2, Line 216: for (ThreadInfo thread : deadlockedThreads)
LOG.error(thread.toString());
> nit: in theory, given the threads are deadlocked, it should be guaranteed t
Done
http://gerrit.cloudera.org:8080/#/c/11317/2/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java@217
PS2, Line 217: LOG.fatal("Aborting because of deadlocked threads in
JVM.");
> Should we dump the entire thread stacks (not just the deadlocked ones) and
Dumping all the other state feels out of scope for this change. It seems like
useful infrastructure but I don't want to do a once-off version of it for this
one case. If we want to add a utility to dump all JVM state then we could add a
call to that here.
http://gerrit.cloudera.org:8080/#/c/11317/2/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java@217
PS2, Line 217: LOG.fatal("Aborting because of deadlocked threads in
JVM.");
> +1 for other thread stacks.
Done
http://gerrit.cloudera.org:8080/#/c/11317/2/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java@217
PS2, Line 217: LOG.fatal("Aborting because of deadlocked threads in
JVM.");
> oh, also, does log4j "fatal" actually guarantee the process exits? You dont
This made use of the fact that we hooked up l
[Impala-ASF-CR] IMPALA-7469. Invalidate LocalCatalog cache based on topic updates
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/11280 )
Change subject: IMPALA-7469. Invalidate LocalCatalog cache based on topic
updates
..
Patch Set 2:
(13 comments)
haven't looked at the tests yet.
http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@422
PS2, Line 422:
: // Serialize a minimal version of the object that can be
used by impalads
: // that are running in 'local-catalog' mode. This is used
by those impalads
: // to invalidate their local cache.
: TCatalogObject minimalObject = getMinimalObjectForV2(obj);
: String v2Key =
CatalogObjectsConstants.CATALOG_TOPIC_V2_PREFIX + key;
: if
(!FeSupport.NativeAddPendingTopicItem(nativeCatalogServerPtr, v2Key,
: obj.catalog_version,
serializer.serialize(minimalObject), delete)) {
: LOG.error("NativeAddPendingTopicItem failed in BE. key="
+ v2Key + ", delete="
: + delete + ", data_size=" + data.length);
: }
Can we avoid these if there are no subscribers subscribed to this prefix?
(which I believe is the most common case until the local catalog is a stable
feature)
Please ignore if it annoying to plumb through the code to get this information.
http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java:
http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@255
PS2, Line 255: public Database loadDb(final String dbName) throws TException {
nit: Add method docs in general. Also worth mentioning that it tries to get
from cache, else makes an RPC ..
http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@263
PS2, Line 263: TGetPartialCatalogObjectRequest req =
newReqForDb(dbName);
hit.setRef(false) (below too), wondering if can make this whole logic of cache
get/call and set hit bit into a method. Its all over the place.
http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@267
PS2, Line 267: );
dbname
http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@290
PS2, Line 290: TGetPartialCatalogObjectRequest req =
newReqForDb(dbName);
hit.setRef(false)
http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@294
PS2, Line 294: expected HMS table"
table names?
http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@344
PS2, Line 344: ");
table name
http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@619
PS2, Line 619: LOG.trace("loadFunctionNames({}): not cached yet", dbName);
This reminds me of
https://github.com/apache/impala/commit/352833b8cfcf5e0246f322fec1ee9b7612e0ed6a
Mostafa once pointed out it can improve performance.
http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@647
PS2, Line 647: catalogServiceId_ = req.catalog_service_id;
I think we should detect restarts here and log
if (catalogServiceId_ ! = INITIAL_VERSION) LOG.warn(Detected a catalog
restart...)
http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@688
PS2, Line 688: anyuthing
nit: typo
http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@735
PS2, Line 735: debug
debug is the default, you mean trace?
http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@747
PS2, Line 747: asMap().remove
isn't invalidate() the right method? Are you using this for return val?
http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@896
PS2, Line 896: VersionedTableCacheKey
Where is this used? It looks like loadTableNames() uses the base TableCacheKey
and hence it is likely to be replaced in the cache. Maybe I'm missing something.
--
To view, visit http://gerrit.cloudera.org:8080/11280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project:
[Impala-ASF-CR] IMPALA-7476: Use context to limit scope of StatestoreSubscriber
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11326 Change subject: IMPALA-7476: Use context to limit scope of StatestoreSubscriber .. IMPALA-7476: Use context to limit scope of StatestoreSubscriber StatestoreSubscribers are outliving the end of test_statestore.py, causing continued interaction with the statestored with a lot of log spew in statestored.INFO and the main test log. This change converts StatestoreSubscriber to use a Python context manager. It updates all the tests to use this context manager. This insures that the StatestoreSubscriber runs kill() at the end of each test. There is also a circular reference between StatestoreSubscriber and KillableThreadedServer that prevents cleanup. This resolves the circular reference by nulling out some variables on KillableThreadedServer::shutdown(). Testing: - Verified log spew is gone - StatestoreSubscriber::__del__ is called for 16 out of 17 tests (exception: test_hung_heartbeat) where before it was called for none Change-Id: Ie71fac6095cd94343f68b586238aed410ebbca39 --- M tests/statestore/test_statestore.py 1 file changed, 135 insertions(+), 123 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/11326/1 -- To view, visit http://gerrit.cloudera.org:8080/11326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ie71fac6095cd94343f68b586238aed410ebbca39 Gerrit-Change-Number: 11326 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell
[Impala-ASF-CR] IMPALA-7476: Use context to limit scope of StatestoreSubscriber
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11326 ) Change subject: IMPALA-7476: Use context to limit scope of StatestoreSubscriber .. Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/11326/1/tests/statestore/test_statestore.py File tests/statestore/test_statestore.py: http://gerrit.cloudera.org:8080/#/c/11326/1/tests/statestore/test_statestore.py@579 PS1, Line 579: . flake8: E131 continuation line unaligned for hanging indent http://gerrit.cloudera.org:8080/#/c/11326/1/tests/statestore/test_statestore.py@616 PS1, Line 616: . flake8: E131 continuation line unaligned for hanging indent http://gerrit.cloudera.org:8080/#/c/11326/1/tests/statestore/test_statestore.py@628 PS1, Line 628: . flake8: E131 continuation line unaligned for hanging indent http://gerrit.cloudera.org:8080/#/c/11326/1/tests/statestore/test_statestore.py@739 PS1, Line 739: . flake8: E131 continuation line unaligned for hanging indent http://gerrit.cloudera.org:8080/#/c/11326/1/tests/statestore/test_statestore.py@743 PS1, Line 743: . flake8: E131 continuation line unaligned for hanging indent -- To view, visit http://gerrit.cloudera.org:8080/11326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie71fac6095cd94343f68b586238aed410ebbca39 Gerrit-Change-Number: 11326 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Sat, 25 Aug 2018 00:39:41 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7477: Batch-oriented query set construction
Hello Zoltan Borok-Nagy, Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11297 to look at the new patch set (#5). Change subject: IMPALA-7477: Batch-oriented query set construction .. IMPALA-7477: Batch-oriented query set construction Rework the row-by-row construction of query result sets in PlanRootSink so that it materialises an output column at a time. Make some minor optimisations like preallocating output vectors and initialising strings more efficiently. My intent is both to make this faster and to make the QueryResultSet interface better before IMPALA-4268 does a bunch of surgery on this part of the code. Testing: Ran core tests. Perf: Downloaded tpch_parquet.orders via JDBC driver. Before: 3.01s, After: 2.57s. Downloaded l_orderkey from tpch_parquet.lineitem. Before: 1.21s, After: 1.08s. Change-Id: Ibc87a84c34935d0d5841c7f5528eb802527fa809 --- M be/src/exec/plan-root-sink.cc M be/src/exec/plan-root-sink.h M be/src/service/hs2-util.cc M be/src/service/hs2-util.h M be/src/service/query-result-set.cc M be/src/service/query-result-set.h 6 files changed, 314 insertions(+), 159 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/11297/5 -- To view, visit http://gerrit.cloudera.org:8080/11297 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibc87a84c34935d0d5841c7f5528eb802527fa809 Gerrit-Change-Number: 11297 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-7476: Use context to limit scope of StatestoreSubscriber
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11326 ) Change subject: IMPALA-7476: Use context to limit scope of StatestoreSubscriber .. Patch Set 1: Code-Review+2 (1 comment) Thanks for fixing this. Looks good to go once the flake8 issues are fixed. http://gerrit.cloudera.org:8080/#/c/11326/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11326/1//COMMIT_MSG@16 PS1, Line 16: insures pedantry: ensures -- To view, visit http://gerrit.cloudera.org:8080/11326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie71fac6095cd94343f68b586238aed410ebbca39 Gerrit-Change-Number: 11326 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 25 Aug 2018 00:47:13 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7476: Use context to limit scope of StatestoreSubscriber
Hello Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11326 to look at the new patch set (#2). Change subject: IMPALA-7476: Use context to limit scope of StatestoreSubscriber .. IMPALA-7476: Use context to limit scope of StatestoreSubscriber StatestoreSubscribers are outliving the end of test_statestore.py, causing continued interaction with the statestored with a lot of log spew in statestored.INFO and the main test log. This change converts StatestoreSubscriber to use a Python context manager. It updates all the tests to use this context manager. This ensures that the StatestoreSubscriber runs kill() at the end of each test. There is also a circular reference between StatestoreSubscriber and KillableThreadedServer that prevents cleanup. This resolves the circular reference by nulling out some variables on KillableThreadedServer::shutdown(). Testing: - Verified log spew is gone - StatestoreSubscriber::__del__ is called for 16 out of 17 tests (exception: test_hung_heartbeat) where before it was called for none Change-Id: Ie71fac6095cd94343f68b586238aed410ebbca39 --- M tests/statestore/test_statestore.py 1 file changed, 135 insertions(+), 123 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/11326/2 -- To view, visit http://gerrit.cloudera.org:8080/11326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie71fac6095cd94343f68b586238aed410ebbca39 Gerrit-Change-Number: 11326 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7476: Use context to limit scope of StatestoreSubscriber
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/11326 ) Change subject: IMPALA-7476: Use context to limit scope of StatestoreSubscriber .. Patch Set 2: Code-Review+2 Carry +2 -- To view, visit http://gerrit.cloudera.org:8080/11326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie71fac6095cd94343f68b586238aed410ebbca39 Gerrit-Change-Number: 11326 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 25 Aug 2018 00:50:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7476: Use context to limit scope of StatestoreSubscriber
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/11326 ) Change subject: IMPALA-7476: Use context to limit scope of StatestoreSubscriber .. Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/11326/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11326/1//COMMIT_MSG@16 PS1, Line 16: ensures > pedantry: ensures Done http://gerrit.cloudera.org:8080/#/c/11326/1/tests/statestore/test_statestore.py File tests/statestore/test_statestore.py: http://gerrit.cloudera.org:8080/#/c/11326/1/tests/statestore/test_statestore.py@579 PS1, Line 579: > flake8: E131 continuation line unaligned for hanging indent Done http://gerrit.cloudera.org:8080/#/c/11326/1/tests/statestore/test_statestore.py@616 PS1, Line 616: > flake8: E131 continuation line unaligned for hanging indent Done http://gerrit.cloudera.org:8080/#/c/11326/1/tests/statestore/test_statestore.py@628 PS1, Line 628: > flake8: E131 continuation line unaligned for hanging indent Done http://gerrit.cloudera.org:8080/#/c/11326/1/tests/statestore/test_statestore.py@739 PS1, Line 739: > flake8: E131 continuation line unaligned for hanging indent Done http://gerrit.cloudera.org:8080/#/c/11326/1/tests/statestore/test_statestore.py@743 PS1, Line 743: > flake8: E131 continuation line unaligned for hanging indent Done -- To view, visit http://gerrit.cloudera.org:8080/11326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie71fac6095cd94343f68b586238aed410ebbca39 Gerrit-Change-Number: 11326 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 25 Aug 2018 00:49:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11317 ) Change subject: IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/480/ : 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/11317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If46f879e7ab9eeead0c6e9a09844e1356012898d Gerrit-Change-Number: 11317 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Sat, 25 Aug 2018 00:51:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7476: Use context to limit scope of StatestoreSubscriber
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11326 ) Change subject: IMPALA-7476: Use context to limit scope of StatestoreSubscriber .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3084/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie71fac6095cd94343f68b586238aed410ebbca39 Gerrit-Change-Number: 11326 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 25 Aug 2018 00:53:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7476: Use context to limit scope of StatestoreSubscriber
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11326 ) Change subject: IMPALA-7476: Use context to limit scope of StatestoreSubscriber .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/481/ : 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/11326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie71fac6095cd94343f68b586238aed410ebbca39 Gerrit-Change-Number: 11326 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 25 Aug 2018 01:17:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7477: Batch-oriented query set construction
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11297 ) Change subject: IMPALA-7477: Batch-oriented query set construction .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/482/ : 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/11297 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibc87a84c34935d0d5841c7f5528eb802527fa809 Gerrit-Change-Number: 11297 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Sat, 25 Aug 2018 01:27:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7476: Use context to limit scope of StatestoreSubscriber
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11326 ) Change subject: IMPALA-7476: Use context to limit scope of StatestoreSubscriber .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/483/ : 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/11326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie71fac6095cd94343f68b586238aed410ebbca39 Gerrit-Change-Number: 11326 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 25 Aug 2018 01:39:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7469. Invalidate LocalCatalog cache based on topic updates
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11280 ) Change subject: IMPALA-7469. Invalidate LocalCatalog cache based on topic updates .. Patch Set 2: (5 comments) comments from initial reading. http://gerrit.cloudera.org:8080/#/c/11280/2/common/thrift/CatalogObjects.thrift File common/thrift/CatalogObjects.thrift: http://gerrit.cloudera.org:8080/#/c/11280/2/common/thrift/CatalogObjects.thrift@621 PS2, Line 621: perhaps it makes more sense to put these in CatalogService.thrift? http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@429 PS2, Line 429: delete is delete ignored for this case? http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/Table.java File fe/src/main/java/org/apache/impala/catalog/Table.java: http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/Table.java@411 PS2, Line 411: // TODO(todd): this breaks test_ddl.test_alter_set_column_stats. Is this the fix here to make that test work, or is that test in a currently non-working state (I didn't see it xfailed)? http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@699 PS2, Line 699: ineffective how is MAX_VALUE handled? is it ignored as a special case? from the description, it would seem that this impalad would wait until it sees MAX_VALUE minVersion, which seems odd. http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@970 PS2, Line 970: TODO(todd): cu this change looks like it covers this todo. -- To view, visit http://gerrit.cloudera.org:8080/11280 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I615f9e6bd167b36cd8d93da59426dd6813ae4984 Gerrit-Change-Number: 11280 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Sat, 25 Aug 2018 01:49:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7421. Static methods use wrong JNI call function
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11181 ) Change subject: IMPALA-7421. Static methods use wrong JNI call function .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11181 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If7cde6ca91613b63afe5307f4d819fb24cb17fd6 Gerrit-Change-Number: 11181 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Sat, 25 Aug 2018 01:51:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/11317 ) Change subject: IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If46f879e7ab9eeead0c6e9a09844e1356012898d Gerrit-Change-Number: 11317 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Sat, 25 Aug 2018 03:15:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
Philip Zeyliger has posted comments on this change. (
http://gerrit.cloudera.org:8080/11317 )
Change subject: IMPALA-7483: abort stuck impalad/catalogd on JVM deadlock
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11317/2/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java
File fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java:
http://gerrit.cloudera.org:8080/#/c/11317/2/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java@217
PS2, Line 217: LOG.fatal("Aborting because of deadlocked threads in
JVM.");
> This made use of the fact that we hooked up log4j to glog. Glog fatal does
I had no idea. I wonder when someone will write a UDF that uses java logging
that triggers this!
--
To view, visit http://gerrit.cloudera.org:8080/11317
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If46f879e7ab9eeead0c6e9a09844e1356012898d
Gerrit-Change-Number: 11317
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Sat, 25 Aug 2018 03:15:06 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7476: Use context to limit scope of StatestoreSubscriber
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11326 ) Change subject: IMPALA-7476: Use context to limit scope of StatestoreSubscriber .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie71fac6095cd94343f68b586238aed410ebbca39 Gerrit-Change-Number: 11326 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 25 Aug 2018 04:18:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11183 ) Change subject: IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions .. Patch Set 11: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3080/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I572b5876b979ddae58165bd40d5b008ce9d7a4aa Gerrit-Change-Number: 11183 Gerrit-PatchSet: 11 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 24 Aug 2018 09:01:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11183 ) Change subject: IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions .. Patch Set 11: The last build failure was caused by a hung test that seemed unrelated. I have created a jira about it: IMPALA-7485 "test_spilling_naaj hung on jenkins" -- To view, visit http://gerrit.cloudera.org:8080/11183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I572b5876b979ddae58165bd40d5b008ce9d7a4aa Gerrit-Change-Number: 11183 Gerrit-PatchSet: 11 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 24 Aug 2018 10:09:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11183 ) Change subject: IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions .. Patch Set 11: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I572b5876b979ddae58165bd40d5b008ce9d7a4aa Gerrit-Change-Number: 11183 Gerrit-PatchSet: 11 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 24 Aug 2018 12:22:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions
Attila Jeges has posted comments on this change. (
http://gerrit.cloudera.org:8080/11183 )
Change subject: IMPALA-7417: Speed up sub-second unix time->TimestampValue
conversions
..
Patch Set 11:
(15 comments)
Thanks! Another batch of comments:
http://gerrit.cloudera.org:8080/#/c/11183/11//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11183/11//COMMIT_MSG@10
PS11, Line 10: (which is split do date_ and time_ similarly to
boost::posix_time::ptime)
nit: please wrap at 70 characters, here and below.
http://gerrit.cloudera.org:8080/#/c/11183/11/be/src/benchmarks/convert-timestamp-benchmark.cc
File be/src/benchmarks/convert-timestamp-benchmark.cc:
http://gerrit.cloudera.org:8080/#/c/11183/11/be/src/benchmarks/convert-timestamp-benchmark.cc@578
PS11, Line 578: boost::posix_time::seconds(time)
It probably wouldn't make a difference but maybe you should call
boost::posix_time::nanoseconds(time*NANOS_PER_SEC) here to be consistent with
TimestampValue::UtcFromUnixTimeTicks().
http://gerrit.cloudera.org:8080/#/c/11183/11/be/src/benchmarks/convert-timestamp-benchmark.cc@631
PS11, Line 631: temp += boost::posix_time::nanoseconds((unix_time -
unix_time_whole) * NANOS_PER_SEC);
The original implementation had:
temp += boost::posix_time::nanoseconds((unix_time - unix_time_whole) /
ONE_BILLIONTH);
Any reason you changed it?
http://gerrit.cloudera.org:8080/#/c/11183/11/be/src/benchmarks/convert-timestamp-benchmark.cc@635
PS11, Line 635: TimestampValue from_subsecond_unix_time_new(
: const double& unix_time) {
: int64_t unix_time_whole = round(unix_time);
: int64_t nanos = round((unix_time -
unix_time_whole)*NANOS_PER_SEC);
: return TimestampValue::FromUnixTimeNanos(
: unix_time_whole, nanos,
TimezoneDatabase::GetUtcTimezone());
: }
The implementation of TimestampValue::FromSubsecondUnixTime() was changed in
patch-set #11. Please change the implementation here too.
http://gerrit.cloudera.org:8080/#/c/11183/11/be/src/benchmarks/convert-timestamp-benchmark.cc@923
PS11, Line 923: // Note that the test data contains only whole seconds. The
results could be slightly
: // different for sub-second times due to the different
rounding rules.
Are the rounding rules still different?
http://gerrit.cloudera.org:8080/#/c/11183/11/be/src/benchmarks/convert-timestamp-benchmark.cc@932
PS11, Line 932: Benchmark from_subsecond_unix_time("FromSubsecondUnixTime");
This benchmark and the previous bm_utc_from_unix_time_nanos benchmark measures
very similar code. Do we need both?
http://gerrit.cloudera.org:8080/#/c/11183/11/be/src/runtime/timestamp-test.cc
File be/src/runtime/timestamp-test.cc:
http://gerrit.cloudera.org:8080/#/c/11183/11/be/src/runtime/timestamp-test.cc@235
PS11, Line 235: Double can express sub-nanoseconds
You mean "'nanos' can express sub-nanoseconds"?
I'm not really sure what is the role of 'nanos' in this test. Could you clarify
in the comment?
http://gerrit.cloudera.org:8080/#/c/11183/11/be/src/runtime/timestamp-test.cc@240
PS11, Line 240: 1000
Maybe use MILLIS_PER_SEC instead?
http://gerrit.cloudera.org:8080/#/c/11183/11/be/src/runtime/timestamp-test.cc@282
PS11, Line 282:
nit: extra space
http://gerrit.cloudera.org:8080/#/c/11183/11/be/src/runtime/timestamp-test.cc@280
PS11, Line 280: // Try the same timestamp with opposite sign.
: seconds += millis < 0 ? -1 : 1;
: millis += millis < 0 ? 1000 : -1000;
: EXPECT_EQ(result, TestFromSubSecondFunctionsInner(seconds,
millis));
Why do we need this extra test? And how is this 'opposite sign'? Please clarify
in the comment.
http://gerrit.cloudera.org:8080/#/c/11183/11/be/src/runtime/timestamp-test.cc@284
PS11, Line 284: if
nit: missing space after 'if'
http://gerrit.cloudera.org:8080/#/c/11183/11/be/src/runtime/timestamp-test.cc@848
PS11, Line 848: Test subsecond unix time conversion for non edge cases.
How about adding some extra tests for the micros/nanos unix time conversion
functions to make sure that micros/nanos precision is handled correctly.
http://gerrit.cloudera.org:8080/#/c/11183/11/be/src/runtime/timestamp-value.h
File be/src/runtime/timestamp-value.h:
http://gerrit.cloudera.org:8080/#/c/11183/11/be/src/runtime/timestamp-value.h@189
PS11, Line 189: time.total_nanoseconds() >= 0
Instead this, you could use !time.is_negative()
http://gerrit.cloudera.org:8080/#/c/11183/11/be/src/runtime/timestamp-value.h@190
PS11, Line 190: time.total_nanoseconds() < NANOS_PER_DAY
maybe use (time.hours() < 24) ?
http://gerrit.cloudera.org:8080/#/c/11183/11/be/src/runtime/timestamp-value.h@328
PS11, Line 328: unix_time
unix_time_ticks
--
To view, visit http://gerrit.cloudera.org:8080/11183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11052 ) Change subject: IMPALA-6644: Add last heartbeat timestamp into Statestore metric .. Patch Set 8: (10 comments) http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.h File be/src/statestore/statestore.h: http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.h@420 PS8, Line 420: /// Sets the subscriber's last heartbeat timestamp to the given value. nit: can you clarify what the reference point for this timestamp is? ie is this a wall time or monotonic time? You might consider using the kudu::MonoTime class here to make that clear in lieu of a comment (not sure if you have a similar wrapper, but that should already be imported in kudu/util) http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.h@445 PS8, Line 445: microseconds same as above http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.h@538 PS8, Line 538: AtomicBool why does this need to be atomic? shouldn't we assume that initialization is single-threaded and the object should not be exposed to other threads until after initialization? http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.cc File be/src/statestore/statestore.cc: http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.cc@77 PS8, Line 77: DEFINE_int32 perhaps this should be _hidden unless we have some use case in mind where an admin would want to tune it? http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.cc@389 PS8, Line 389: DCHECK_GT(timestamp_us, last_heartbeat_ts_.Load()); should this be _GE? it's possible (though unlikely) that we get called in rapid succession. http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.cc@445 PS8, Line 445: if (initialized_.Load()) LOG(FATAL) this can just be CHECK(!initialized_.Load()) http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.cc@552 PS8, Line 552: time_since_heartbeat can you name this secs_since_heartbeat so the unit is clear? http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.cc@916 PS8, Line 916: SetLastHeartbeatTimestamp(MonotonicMicros()); Given the argument to this method is always going to be MonotonicMicros(), what if the method was just called 'RefreshHeartbeatTimestamp()' or something and internally it called MonotonicMicros? Then, instead of the getter, you could have 'double TimeSinceHeartbeaat()' which calculates the subtraction. That would encapsulate the timing mechanism and units internal to the Subscriber class and only expose a minimal interface to callers http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.cc@1002 PS8, Line 1002: .size() == 0 empty http://gerrit.cloudera.org:8080/#/c/11052/8/www/statestore_subscribers.tmpl File www/statestore_subscribers.tmpl: http://gerrit.cloudera.org:8080/#/c/11052/8/www/statestore_subscribers.tmpl@31 PS8, Line 31: Time since last heartbeat (seconds) more concise: "Seconds since last heartbeat" -- To view, visit http://gerrit.cloudera.org:8080/11052 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Gerrit-Change-Number: 11052 Gerrit-PatchSet: 8 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 24 Aug 2018 16:15:13 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Lars Volker has posted comments on this change. (
http://gerrit.cloudera.org:8080/10847 )
Change subject: IMPALA-6271: Impala daemon should log a message when it's being
shut down
..
Patch Set 12:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc
File be/src/common/init.cc:
http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc@297
PS10, Line 297: if (sigaction(SIGTERM, &action, nullptr) == -1) {
> There is a value 1 displayed for old_action.sa_handler ...
Where is it displayed? Should we validate that it's this value then?
http://gerrit.cloudera.org:8080/#/c/10847/12/be/src/common/init.cc
File be/src/common/init.cc:
http://gerrit.cloudera.org:8080/#/c/10847/12/be/src/common/init.cc@298
PS12, Line 298: "
> Should we also print the error itself then, e.g. using GetStrErrMsg()?
Ping?
--
To view, visit http://gerrit.cloudera.org:8080/10847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29
Gerrit-Change-Number: 10847
Gerrit-PatchSet: 12
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Zoram Thanga
Gerrit-Comment-Date: Fri, 24 Aug 2018 16:19:23 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7425: Change incremental stats to pull from catalogd.
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/11193 )
Change subject: IMPALA-7425: Change incremental stats to pull from catalogd.
..
Patch Set 2:
(10 comments)
only got partially through but have some meetings for the rest of the morning
and early afternoon so figured i'd send along what i got so far
http://gerrit.cloudera.org:8080/#/c/11193/2/common/thrift/CatalogService.thrift
File common/thrift/CatalogService.thrift:
http://gerrit.cloudera.org:8080/#/c/11193/2/common/thrift/CatalogService.thrift@379
PS2, Line 379: statistcs
typo
http://gerrit.cloudera.org:8080/#/c/11193/2/common/thrift/CatalogService.thrift@382
PS2, Line 382: 1: required CatalogObjects.TTableName table_name
: 2: required list partition_ids
it seems risky to use partition IDs without being tied to a particular version
number of the table. If there is a concurrent refresh or reload of the table,
we'd end up pulling stats for the wrong partitions or incorrectly deciding that
there were no stats available when there were. Partition names are a more
stable identifier
http://gerrit.cloudera.org:8080/#/c/11193/2/common/thrift/CatalogService.thrift@387
PS2, Line 387: statistcs
typo
http://gerrit.cloudera.org:8080/#/c/11193/2/common/thrift/CatalogService.thrift@388
PS2, Line 388: // If there was an error, an empty partition_stats mapping is
returned along with
- seems like it would be easier to just make the returned map optional so youu
dont need to return an empty one in the error case.
- can you clarify what happens when you request stats for a partition that does
not have them? Do you get a map entry or just a "missing" entry?
http://gerrit.cloudera.org:8080/#/c/11193/2/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
File fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java:
http://gerrit.cloudera.org:8080/#/c/11193/2/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java@336
PS2, Line 336: LOG.info("analyzing compute stats");
probably didn't mean to leave this in at INFO level
http://gerrit.cloudera.org:8080/#/c/11193/2/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java@451
PS2, Line 451: !RuntimeEnv.INSTANCE.isTestEnv()
why not isTestEnv?
http://gerrit.cloudera.org:8080/#/c/11193/2/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java@456
PS2, Line 456: fetchedPartitionStats = fetchPartitionStats(analyzer,
partStatsToFetch);
it seems some of this code ends up duplicated in a few places. Did you consider
adding a flag to 'loadPartitions' and 'loadAllPartitions' which says
'NEED_INCREMENTAL_STATS'?
Or maybe this code could end up being a little cleaner if you always follow the
same code path to "fetch stats" into 'fetchedPartitionStats', but in the "old"
code path, you just "fetch" them by copying the reference from the partition
object? I think that could reduce the branching factor of the code below and
allow you to consolidate all the code into 'fetchPartitionStats'
http://gerrit.cloudera.org:8080/#/c/11193/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/11193/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@415
PS2, Line 415: // Table is not loaded.
: if (!table.isLoaded()) {
: throw new CatalogException("Partition statistics not
available for not yet "
: + " loaded table: " + request.table_name);
can this be the case? I thought if it's FeFsTable then it's guaranteed to be
loaded
http://gerrit.cloudera.org:8080/#/c/11193/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@428
PS2, Line 428: tableName.getDb_name() + "." + tableName.getTable_name()
nit: fsTable.getFullName()
http://gerrit.cloudera.org:8080/#/c/11193/2/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java:
http://gerrit.cloudera.org:8080/#/c/11193/2/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@524
PS2, Line 524: may not be serialized and sent from catalogd to impalads.
I find this usage of the word "may" unclear -- not sure if you mean "it's not
permitted" or "it is possible that". Can you rephrase to something like
"Depending on configuration, it is possible that incremental stats are not
sent" or somesuch?
--
To view, visit http://gerrit.cloudera.org:8080/11193
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d564808ca5157afe4e091909ca6cdac76e60d6e
Gerrit-Change-Number: 11193
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: I
[Impala-ASF-CR] IMPALA-7349: Add Admission control support for automatically setting mem limit
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11157 ) Change subject: IMPALA-7349: Add Admission control support for automatically setting mem_limit .. Patch Set 7: (6 comments) http://gerrit.cloudera.org:8080/#/c/11157/7/be/src/scheduling/admission-controller.h File be/src/scheduling/admission-controller.h: http://gerrit.cloudera.org:8080/#/c/11157/7/be/src/scheduling/admission-controller.h@511 PS7, Line 511: False nit: false http://gerrit.cloudera.org:8080/#/c/11157/7/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/11157/7/be/src/scheduling/admission-controller.cc@326 PS7, Line 326: DCHECK_NE DCHECK_GT? http://gerrit.cloudera.org:8080/#/c/11157/7/be/src/scheduling/admission-controller.cc@370 PS7, Line 370: ComputePerHostMemLimit Maybe we should call this ComputePerHostMemToAdmit()? I think the local variable name is more descriptive than the function name. http://gerrit.cloudera.org:8080/#/c/11157/7/be/src/scheduling/admission-controller.cc@372 PS7, Line 372: int64_t cluster_mem_to_admit = per_host_mem_to_admit * schedule.GetNumBackends(); I filed IMPALA-7486 as a follow-on to think about generalising this. I don't think now is the right time to do it but wanted to track it. http://gerrit.cloudera.org:8080/#/c/11157/7/be/src/scheduling/admission-controller.cc@527 PS7, Line 527: per_host_mem_estimate nit: I don't think "estimate" is the right term here. http://gerrit.cloudera.org:8080/#/c/11157/7/be/src/scheduling/admission-controller.cc@527 PS7, Line 527: ComputePerHostMemLimit thought: maybe modify this function to return a struct with both the per-host and cluster memory to avoid duplicating the cluster calculation -- To view, visit http://gerrit.cloudera.org:8080/11157 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifec00141651982f5975803c2165b7d7a10ebeaa6 Gerrit-Change-Number: 11157 Gerrit-PatchSet: 7 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 24 Aug 2018 16:57:23 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7477: Batch-oriented query set construction
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11297 Change subject: IMPALA-7477: Batch-oriented query set construction .. IMPALA-7477: Batch-oriented query set construction Rework the row-by-row construction of query result sets in PlanRootSink so that it materialises an output column at a time. Make some minor optimisations like preallocating output vectors and initialising strings more efficiently. My intent is both to make this faster and to make the QueryResultSet interface better before IMPALA-4268 does a bunch of surgery on this part of the code. Testing: Ran core tests. Perf: Downloaded tpch_parquet.orders via JDBC driver. Before: 3.01s, After: 2.57s. Downloaded l_orderkey from tpch_parquet.lineitem. Before: 1.21s, After: 1.08s. Change-Id: Ibc87a84c34935d0d5841c7f5528eb802527fa809 --- M be/src/exec/plan-root-sink.cc M be/src/exec/plan-root-sink.h M be/src/service/hs2-util.cc M be/src/service/hs2-util.h M be/src/service/query-result-set.cc M be/src/service/query-result-set.h 6 files changed, 305 insertions(+), 158 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/11297/4 -- To view, visit http://gerrit.cloudera.org:8080/11297 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ibc87a84c34935d0d5841c7f5528eb802527fa809 Gerrit-Change-Number: 11297 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-110: Support for multiple DISTINCT
Vuk Ercegovac has posted comments on this change. (
http://gerrit.cloudera.org:8080/10771 )
Change subject: IMPALA-110: Support for multiple DISTINCT
..
Patch Set 7:
(25 comments)
comments from initial pass. still looking at several backend classes, some of
the tests, and multi-aggregate info.
http://gerrit.cloudera.org:8080/#/c/10771/7/be/src/exec/aggregation-node-base.cc
File be/src/exec/aggregation-node-base.cc:
http://gerrit.cloudera.org:8080/#/c/10771/7/be/src/exec/aggregation-node-base.cc@85
PS7, Line 85: fast path to avoid going through all tuples
optimization that bets that the index of non-null agg will be the same from one
tuple to the next.
http://gerrit.cloudera.org:8080/#/c/10771/6/be/src/exec/aggregator.h
File be/src/exec/aggregator.h:
http://gerrit.cloudera.org:8080/#/c/10771/6/be/src/exec/aggregator.h@64
PS6, Line 64: int agg_idx
add a comment mentioning what this index is relative to
http://gerrit.cloudera.org:8080/#/c/10771/6/be/src/exprs/valid-tuple-id.h
File be/src/exprs/valid-tuple-id.h:
http://gerrit.cloudera.org:8080/#/c/10771/6/be/src/exprs/valid-tuple-id.h@28
PS6, Line 28: /// Valid input rows must have exactly one non-NULL tuple.
what happens if it doesn't?
http://gerrit.cloudera.org:8080/#/c/10771/6/be/src/runtime/row-batch.h
File be/src/runtime/row-batch.h:
http://gerrit.cloudera.org:8080/#/c/10771/6/be/src/runtime/row-batch.h@247
PS6, Line 247: int RowNum() { return (row_ - parent_->tuple_ptrs_) /
num_tuples_per_row_; }
this can't be < 0 or produce a div zero error?
http://gerrit.cloudera.org:8080/#/c/10771/6/common/thrift/PlanNodes.thrift
File common/thrift/PlanNodes.thrift:
http://gerrit.cloudera.org:8080/#/c/10771/6/common/thrift/PlanNodes.thrift@401
PS6, Line 401: replicate_input
add a comment for this
http://gerrit.cloudera.org:8080/#/c/10771/7/common/thrift/PlanNodes.thrift
File common/thrift/PlanNodes.thrift:
http://gerrit.cloudera.org:8080/#/c/10771/7/common/thrift/PlanNodes.thrift@397
PS7, Line 397:
pls add comments for this one (in particular, replicate_input)
http://gerrit.cloudera.org:8080/#/c/10771/6/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
File fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java:
http://gerrit.cloudera.org:8080/#/c/10771/6/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java@38
PS6, Line 38: A mix of distinct and non-distinct aggregation functions is
allowed as long as all
: * distinct functions have the same distinct expressions.
I did not understand this-- clarify with a counter-example.
http://gerrit.cloudera.org:8080/#/c/10771/7/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
File fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java:
http://gerrit.cloudera.org:8080/#/c/10771/7/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@50
PS7, Line 50: * - Only a non-distinct class:
add an example, such as: max(a)
http://gerrit.cloudera.org:8080/#/c/10771/7/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@52
PS7, Line 52: * - One distinct class, and optionally a non-distinct class:
example, count(distinct a)[, max(b)]
http://gerrit.cloudera.org:8080/#/c/10771/7/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@56
PS7, Line 56: * - Multiple distinct classes, and optionally a non-distinct
class
example: count(distinct a), count(distinct b)[, max(c)]
http://gerrit.cloudera.org:8080/#/c/10771/7/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@233
PS7, Line 233: DistinctAggExprs.isEmpty
ideally, this reads without negations... I see three there. perhaps
hasNonDistinctAggExprs?
http://gerrit.cloudera.org:8080/#/c/10771/7/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@314
PS7, Line 314:* - Result tuple id 1 having 3 slots with ids 3, 4, 5
relate these to the columns/exprs in the example.
http://gerrit.cloudera.org:8080/#/c/10771/6/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:
http://gerrit.cloudera.org:8080/#/c/10771/6/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@66
PS6, Line 66: MultiAggregateInfo
does a single aggregate here boil down to a single AggregateInfo?
http://gerrit.cloudera.org:8080/#/c/10771/6/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@387
PS6, Line 387: multiAggInfo_.materializeRequiredSlots(analyzer,
baseTblSmap_);
where is having dealt with now?
http://gerrit.cloudera.org:8080/#/c/10771/7/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:
http://gerrit.cloudera.org:8080/#/c/10771/7/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@387
PS7, Line 387: multiAggInfo_.materializeRequiredSlots(analyzer,
baseTblSmap_);
any idea why it
[Impala-ASF-CR] IMPALA-6923: Remove create database.py and perf result datastore.py
Nithya Janarthanan has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11315 Change subject: IMPALA-6923: Remove create_database.py and perf_result_datastore.py .. IMPALA-6923: Remove create_database.py and perf_result_datastore.py Description: The current upstream usage indicates that report_benchmark_results.py script is the only script in this folder that is used to generate a report comparing performance between two given runs of the performance tests. The other two scripts create_database.py and perf_result_datastore.py store some metrics from a performance test run to database on a specified impala instance. However they rely on some Internal resources not available to external apache community to generate a meaningful interpretation/report of the metrics stored in the performance database. This change also removes lines of code from report_benchmark_results.py that were previously parsing command line options, that were being used by the two scripts that are being removed. Change-Id: I2c9b04f0d0e8ca221e383aab2ba3ee53d968dd10 --- D tests/benchmark/create_database.py D tests/benchmark/perf_result_datastore.py M tests/benchmark/report_benchmark_results.py 3 files changed, 15 insertions(+), 417 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/11315/1 -- To view, visit http://gerrit.cloudera.org:8080/11315 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I2c9b04f0d0e8ca221e383aab2ba3ee53d968dd10 Gerrit-Change-Number: 11315 Gerrit-PatchSet: 1 Gerrit-Owner: Nithya Janarthanan
[Impala-ASF-CR] IMPALA-7477: Batch-oriented query set construction
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11297 ) Change subject: IMPALA-7477: Batch-oriented query set construction .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/471/ : 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/11297 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibc87a84c34935d0d5841c7f5528eb802527fa809 Gerrit-Change-Number: 11297 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 24 Aug 2018 17:33:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6923: Remove create database.py and perf result datastore.py
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11315 ) Change subject: IMPALA-6923: Remove create_database.py and perf_result_datastore.py .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/472/ : 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/11315 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2c9b04f0d0e8ca221e383aab2ba3ee53d968dd10 Gerrit-Change-Number: 11315 Gerrit-PatchSet: 1 Gerrit-Owner: Nithya Janarthanan Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 24 Aug 2018 17:43:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6923: Remove create database.py and perf result datastore.py
Nithya Janarthanan has abandoned this change. ( http://gerrit.cloudera.org:8080/10100 ) Change subject: IMPALA-6923: Remove create_database.py and perf_result_datastore.py .. Abandoned Abandoning this request to start a new one -- To view, visit http://gerrit.cloudera.org:8080/10100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: Ica7c00ad59963d466bae9e607a4692af0138962c Gerrit-Change-Number: 10100 Gerrit-PatchSet: 22 Gerrit-Owner: Nithya Janarthanan Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Nithya Janarthanan
[Impala-ASF-CR] IMPALA-7356 (part 2 of ?): restrict number of coordinators
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11316 Change subject: IMPALA-7356 (part 2 of ?): restrict number of coordinators .. IMPALA-7356 (part 2 of ?): restrict number of coordinators The immediate motivation is to allow testing admission control with a single coordinator where distributed overadmission is not possible. Testing: Ran locally against a 3 node minicluster with max-coordinators unset and set to 1 and 3. Confirmed that queries were submitted to the expected impalads in all cases. Change-Id: I43a6f095fc5274bf649833e0324815ebd266c0e2 --- M tests/stress/concurrent_select.py 1 file changed, 10 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/11316/1 -- To view, visit http://gerrit.cloudera.org:8080/11316 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I43a6f095fc5274bf649833e0324815ebd266c0e2 Gerrit-Change-Number: 11316 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-7356 (part 2 of ?): restrict number of coordinators
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/11316 ) Change subject: IMPALA-7356 (part 2 of ?): restrict number of coordinators .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11316 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I43a6f095fc5274bf649833e0324815ebd266c0e2 Gerrit-Change-Number: 11316 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Comment-Date: Fri, 24 Aug 2018 19:01:12 + Gerrit-HasComments: No
