[Impala-ASF-CR] IMPALA-5552: Add support for authorized proxy groups
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/10510 )
Change subject: IMPALA-5552: Add support for authorized proxy groups
..
Patch Set 8:
(13 comments)
Just came across this CR. I have some comments. Will look at tests in the next
round.
http://gerrit.cloudera.org:8080/#/c/10510/8/be/src/service/impala-server.h
File be/src/service/impala-server.h:
http://gerrit.cloudera.org:8080/#/c/10510/8/be/src/service/impala-server.h@819
PS8, Line 819: /// Adds authorized proxy config into the given map.
Maybe a little more verbose? The context is not clear by just reading this
comment
/// Utility to parse --foo and --bar and populate .
http://gerrit.cloudera.org:8080/#/c/10510/8/be/src/service/impala-server.h@820
PS8, Line 820: AddAuthorizedProxyConfig
nit: How about we call it PopulateAuthorizedProxyConfig()?
http://gerrit.cloudera.org:8080/#/c/10510/8/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/10510/8/be/src/service/impala-server.cc@320
PS8, Line 320: [](const string& config) {
: return Substitute("Invalid proxy user configuration.
No mapping value "
: "specified for the proxy user. For more
information review usage of the "
: "--authorized_proxy_user_config flag: $0", config);
: }
I think using 'Status' as the return of AddAuthorizedProxyConfig() is an Impala
way of doing this (caller checks for !status.ok()). I don't see why we should
unnecessarily complicate this function signature with a lambda. thoughts?
http://gerrit.cloudera.org:8080/#/c/10510/8/be/src/service/impala-server.cc@401
PS8, Line 401: const string& authorized_proxy_config,
: const string& authorized_proxy_config_delimiter
Input params precede output vals [1]. Also we generally use output params as
pointers.
[1] https://google.github.io/styleguide/cppguide.html#Output_Parameters
http://gerrit.cloudera.org:8080/#/c/10510/8/be/src/service/impala-server.cc@425
PS8, Line 425: parsed_allowed_users_or_groups.begin(),
parsed_allowed_users_or_groups.end());
nit: should we check for duplicate values instead of overwriting? ex:
foo=bar;foo=car
http://gerrit.cloudera.org:8080/#/c/10510/8/be/src/service/impala-server.cc@1386
PS8, Line 1386: TGetHadoopGroupsRequest req;
Could you add a comment here that breiefly summarizes what we are doing
http://gerrit.cloudera.org:8080/#/c/10510/8/be/src/service/impala-server.cc@1389
PS8, Line 1389: GetHadoopGroups
I have seen issues around "GroupMapping" (especially with LDAP etc) taking
longer than expected in the HDFS land. I'd assume we can run into similar
issues, especially if we are calling this for every OpenSession(). Any thoughts
on what we can do to help debug such issues without having to take thread dumps
and then figuring it out.
http://gerrit.cloudera.org:8080/#/c/10510/8/be/src/service/impala-server.cc@1391
PS8, Line 1391: There was an
nit: remove
http://gerrit.cloudera.org:8080/#/c/10510/8/be/src/service/impala-server.cc@1397
PS8, Line 1397: AuthorizedProxyMap::const_iterator proxy_group =
: authorized_proxy_group_config_.find(short_user);
: if (proxy_group != authorized_proxy_group_config_.end()) {
Move it outside of the loop? Also can we bypass the GetHadoopGroups() if this
fails in the first place?
http://gerrit.cloudera.org:8080/#/c/10510/8/be/src/service/impala-server.cc@1400
PS8, Line 1400: for (const string& group : proxy_group->second) {
: if (group == "*" || group == do_as_group) return
Status::OK();
: }
Why are we looping here? Isn't this a set? Could we find() in O(1) ?
http://gerrit.cloudera.org:8080/#/c/10510/8/fe/src/main/java/org/apache/impala/service/JniFrontend.java
File fe/src/main/java/org/apache/impala/service/JniFrontend.java:
http://gerrit.cloudera.org:8080/#/c/10510/8/fe/src/main/java/org/apache/impala/service/JniFrontend.java@585
PS8, Line 585: GROUPS
Is this thread safe?
http://gerrit.cloudera.org:8080/#/c/10510/8/fe/src/main/java/org/apache/impala/service/JniFrontend.java@641
PS8, Line 641: throw new InternalException(e.getMessage());
Can you LOG the full exception here. Helps debug issues with group mapping not
working as expected.
http://gerrit.cloudera.org:8080/#/c/10510/8/fe/src/main/java/org/apache/impala/service/JniFrontend.java@672
PS8, Line 672: if (provider instanceof ShellBasedUnixGroupsMapping ||
: provider instanceof ShellBasedUnixGroupsNetgroupMapping)
{
Please add comments as to why these are problematic.
--
To view, visit http://gerrit.cloudera.org:8080/10510
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType:
[Impala-ASF-CR] IMPALA-7128 (part 1) Refactor interfaces for Db, View, Table, Partition
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10611 ) Change subject: IMPALA-7128 (part 1) Refactor interfaces for Db, View, Table, Partition .. Patch Set 1: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2604/ -- To view, visit http://gerrit.cloudera.org:8080/10611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id55f7d2e94d81e66ce720acb6315f15a89621b31 Gerrit-Change-Number: 10611 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 06 Jun 2018 04:54:39 + Gerrit-HasComments: No
[Impala-ASF-CR] Fix Indents from IMPALA-4970
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/10560 ) Change subject: Fix Indents from IMPALA-4970 .. Patch Set 3: Verified+1 > It's a whitespace-only change and it compiled. Thanks for taking care of this, Tim. -- To view, visit http://gerrit.cloudera.org:8080/10560 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4ea53e0780cca9f791808c2a73c7f72ba27c2159 Gerrit-Change-Number: 10560 Gerrit-PatchSet: 3 Gerrit-Owner: Rahul Shivu Mahadev Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Rahul Shivu Mahadev Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 06 Jun 2018 03:48:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7089: reenable test kudu dml reporting
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10577 ) Change subject: IMPALA-7089: reenable test_kudu_dml_reporting .. Patch Set 2: Code-Review+2 Let's get this test coverage back now that we've fixed the root cause. -- To view, visit http://gerrit.cloudera.org:8080/10577 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib5716f30458eb6db08f735bcbd2f79d205334930 Gerrit-Change-Number: 10577 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 06 Jun 2018 03:45:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7089: reenable test kudu dml reporting
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10577 ) Change subject: IMPALA-7089: reenable test_kudu_dml_reporting .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2606/ -- To view, visit http://gerrit.cloudera.org:8080/10577 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib5716f30458eb6db08f735bcbd2f79d205334930 Gerrit-Change-Number: 10577 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 06 Jun 2018 03:45:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7078: Part 1: improve memory consumption of wide Avro scans
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10550 ) Change subject: IMPALA-7078: Part 1: improve memory consumption of wide Avro scans .. Patch Set 11: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2605/ -- To view, visit http://gerrit.cloudera.org:8080/10550 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iebd2600b4784fd19696c9b92eefb7d7e9db0c80b Gerrit-Change-Number: 10550 Gerrit-PatchSet: 11 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 06 Jun 2018 03:42:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7110. Fix errors from error-prone
Tim Armstrong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10583 ) Change subject: IMPALA-7110. Fix errors from error-prone .. IMPALA-7110. Fix errors from error-prone * Fixes several cases of Preconditions.checkNotNull() which were ineffective. Instead, we should use Preconditions.checkState. * Fixed a case of synchronizing on a non-final class member, which is a bad practice. * Fixed a case of trying to stringify an array instead of using Joiner to join it to a human-readable representation. * Fixed a couple cases where the format string in String.format didn't match the number of arguments passed. * Fixed a test where PrimitiveType objects were being used to look up elements in a Map. This would always return null, so the test was ineffective. I elected not to enable error-prone in the default maven profile since it increases compilation time noticeably. It can be enabled by running 'mvn -Perrorprone'. Even with this patch there are a bunch of warnings -- this only addresses the issues that errorprone considers "errors". Change-Id: Ie9bfadecb5b92bba4fc7921a6f87f249ed90e771 Reviewed-on: http://gerrit.cloudera.org:8080/10583 Reviewed-by: Bharath Vissapragada Reviewed-by: Philip Zeyliger Tested-by: Impala Public Jenkins --- M fe/pom.xml M fe/src/main/java/org/apache/impala/analysis/CreateUdfStmt.java M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java M fe/src/main/java/org/apache/impala/analysis/Path.java M fe/src/main/java/org/apache/impala/catalog/DiskIdMapper.java M fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java M fe/src/main/java/org/apache/impala/catalog/StructType.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java 11 files changed, 51 insertions(+), 14 deletions(-) Approvals: Bharath Vissapragada: Looks good to me, but someone else must approve Philip Zeyliger: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10583 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ie9bfadecb5b92bba4fc7921a6f87f249ed90e771 Gerrit-Change-Number: 10583 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] Fix Indents from IMPALA-4970
Tim Armstrong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10560 ) Change subject: Fix Indents from IMPALA-4970 .. Fix Indents from IMPALA-4970 fixing the mistake in indentation made previously Change-Id: I4ea53e0780cca9f791808c2a73c7f72ba27c2159 Reviewed-on: http://gerrit.cloudera.org:8080/10560 Reviewed-by: Sailesh Mukil Tested-by: Tim Armstrong --- M be/src/runtime/coordinator.cc 1 file changed, 4 insertions(+), 4 deletions(-) Approvals: Sailesh Mukil: Looks good to me, approved Tim Armstrong: Verified -- To view, visit http://gerrit.cloudera.org:8080/10560 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I4ea53e0780cca9f791808c2a73c7f72ba27c2159 Gerrit-Change-Number: 10560 Gerrit-PatchSet: 3 Gerrit-Owner: Rahul Shivu Mahadev Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Rahul Shivu Mahadev Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] Fix Indents from IMPALA-4970
Tim Armstrong has removed a vote on this change. Change subject: Fix Indents from IMPALA-4970 .. Removed Verified-1 by Impala Public Jenkins -- To view, visit http://gerrit.cloudera.org:8080/10560 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I4ea53e0780cca9f791808c2a73c7f72ba27c2159 Gerrit-Change-Number: 10560 Gerrit-PatchSet: 2 Gerrit-Owner: Rahul Shivu Mahadev Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Rahul Shivu Mahadev Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6956: deflake and logging for query expiration test
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10602 ) Change subject: IMPALA-6956: deflake and logging for query_expiration test .. IMPALA-6956: deflake and logging for query_expiration test There was a recent flake where the number of in-flight queries that were executing differed from the expected number. The reason for the false negative is that one of the queries expired before the check for in-flight queries: it took too long to issue the queries. This change modifies the timeout for the expired query to not expire so quickly. Additional logging is added to the check for in-flight queries so that we can distinguish the case of too few queries vs. queries that have the wrong state. Change-Id: I01a8762d28ad920b9ec8a0b1b82469618c66768f Reviewed-on: http://gerrit.cloudera.org:8080/10602 Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins --- M tests/custom_cluster/test_query_expiration.py 1 file changed, 9 insertions(+), 6 deletions(-) Approvals: Tim Armstrong: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10602 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I01a8762d28ad920b9ec8a0b1b82469618c66768f Gerrit-Change-Number: 10602 Gerrit-PatchSet: 3 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6956: deflake and logging for query expiration test
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10602 ) Change subject: IMPALA-6956: deflake and logging for query_expiration test .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10602 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I01a8762d28ad920b9ec8a0b1b82469618c66768f Gerrit-Change-Number: 10602 Gerrit-PatchSet: 2 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 06 Jun 2018 03:19:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7105. Ensure fe tests pass when running standalone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10570 ) Change subject: IMPALA-7105. Ensure fe tests pass when running standalone .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10570 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I85286ba44f03526b487212a8ddadf71a9f427555 Gerrit-Change-Number: 10570 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 06 Jun 2018 01:58:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7105. Ensure fe tests pass when running standalone
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10570 ) Change subject: IMPALA-7105. Ensure fe tests pass when running standalone .. IMPALA-7105. Ensure fe tests pass when running standalone CatalogTest and HdfsStorageDescriptorTest would fail when run individually because they relied on various initialization that only occurs when FeSupport is loaded. These tests weren't explicitly loading FeSupport, but instead relying on earlier tests in 'mvn test' to do so. Tested that, with this change, I'm able to run these tests in Eclipse and they pass. I also tested 'mvn test -DreuseForks=false' and verified that all tests now pass in this mode. Change-Id: I85286ba44f03526b487212a8ddadf71a9f427555 Reviewed-on: http://gerrit.cloudera.org:8080/10570 Reviewed-by: Vuk Ercegovac Tested-by: Impala Public Jenkins --- M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java M fe/src/test/java/org/apache/impala/catalog/HdfsStorageDescriptorTest.java M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java 4 files changed, 14 insertions(+), 3 deletions(-) Approvals: Vuk Ercegovac: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10570 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I85286ba44f03526b487212a8ddadf71a9f427555 Gerrit-Change-Number: 10570 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7110. Fix errors from error-prone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10583 ) Change subject: IMPALA-7110. Fix errors from error-prone .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10583 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie9bfadecb5b92bba4fc7921a6f87f249ed90e771 Gerrit-Change-Number: 10583 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 06 Jun 2018 01:46:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2782: part 2 - Allow impala-shell to connect directly to impalad when configured with load balancer and kerberos.
Hello a...@phdata.io, Philip Zeyliger, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10580 to look at the new patch set (#2). Change subject: IMPALA-2782: part 2 - Allow impala-shell to connect directly to impalad when configured with load balancer and kerberos. .. IMPALA-2782: part 2 - Allow impala-shell to connect directly to impalad when configured with load balancer and kerberos. After some additional testing around IMPALA-2782, it was discovered that the impala-shell starts the session displaying the expected hostname (as passed -i flag) on the prompt. This gives the impression that the load balancer was bypassed, however the actual TSSLSocket was still created with the hostname passed into the -b or --kerberos_host_fqdn flag. This change ensures that the hostname used to create the TSSLSocket will always be the one passed in via the -i flag on impala-shell, regardless of whether the -b flag is set. Change-Id: Ibee05bd0dbe8c6ae108b890f0ae0f6900149773a --- M shell/impala_client.py 1 file changed, 7 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/80/10580/2 -- To view, visit http://gerrit.cloudera.org:8080/10580 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibee05bd0dbe8c6ae108b890f0ae0f6900149773a Gerrit-Change-Number: 10580 Gerrit-PatchSet: 2 Gerrit-Owner: Vincent Tran Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: a...@phdata.io
[Impala-ASF-CR] IMPALA-7128 (part 1) Refactor interfaces for Db, View, Table, Partition
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10611 ) Change subject: IMPALA-7128 (part 1) Refactor interfaces for Db, View, Table, Partition .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2604/ -- To view, visit http://gerrit.cloudera.org:8080/10611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id55f7d2e94d81e66ce720acb6315f15a89621b31 Gerrit-Change-Number: 10611 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 06 Jun 2018 01:26:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7128 (part 1) Refactor interfaces for Db, View, Table, Partition
Todd Lipcon has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10611 Change subject: IMPALA-7128 (part 1) Refactor interfaces for Db, View, Table, Partition .. IMPALA-7128 (part 1) Refactor interfaces for Db, View, Table, Partition This refactors out interfaces in the frontend for the interaction between the analysis/planning code and the classes that implement these catalog objects. This takes care of the most commonly used objects but defers some others (e.g. functions, cache pools, data sources, etc) to follow-on patches. There are a few spots remaining in the frontend that still downcast to implementation classes, particularly where the frontend actually makes modifications to catalog objects in-place. I left TODOs in those spots and will come back later as necessary. Change-Id: Id55f7d2e94d81e66ce720acb6315f15a89621b31 --- M fe/src/main/java/org/apache/impala/analysis/AlterTableAddDropRangePartitionStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableAddReplaceColsStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableDropColStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewRenameStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetCachedStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetFileFormatStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java M fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java M fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/IsNullPredicate.java M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java M fe/src/main/java/org/apache/impala/analysis/PartitionDef.java M fe/src/main/java/org/apache/impala/analysis/PartitionSet.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java M fe/src/main/java/org/apache/impala/analysis/Path.java M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/main/java/org/apache/impala/analysis/ShowCreateFunctionStmt.java M fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java M fe/src/main/java/org/apache/impala/analysis/ShowFilesStmt.java M fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java M fe/src/main/java/org/apache/impala/analysis/SlotRef.java M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java M fe/src/main/java/org/apache/impala/analysis/TableDef.java M fe/src/main/java/org/apache/impala/analysis/TableRef.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/analysis/WithClause.java M fe/src/main/java/org/apache/impala/catalog/Catalog.java M fe/src/main/java/org/apache/impala/catalog/Db.java A fe/src/main/java/org/apache/impala/catalog/FeCatalog.java A fe/src/main/java/org/apache/impala/catalog/FeDb.java A fe/src/main/java/org/apache/impala/catalog/FeHdfsPartition.java A fe/src/main/java/org/apache/impala/catalog/FeHdfsTable.java A
[Impala-ASF-CR] IMPALA-7016: Implement ALTER DATABASE SET OWNER
Fredy Wijaya has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/10471 ) Change subject: IMPALA-7016: Implement ALTER DATABASE SET OWNER .. IMPALA-7016: Implement ALTER DATABASE SET OWNER Alters the database owner to either user or role. Syntax: ALTER DATABASE db SET OWNER USER user ALTER DATABASE db SET OWNER ROLE role Testing: - Added new front-end tests - Added new end-to-end tests Change-Id: Ie3b923021ebce5192d2d64784e7ddb952ba82bc3 --- M common/thrift/CatalogService.thrift M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup A fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java A fe/src/main/java/org/apache/impala/analysis/AlterDbStmt.java M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java A fe/src/main/java/org/apache/impala/analysis/Owner.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M tests/metadata/test_ddl.py M tests/metadata/test_ddl_base.py 14 files changed, 328 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/10471/5 -- To view, visit http://gerrit.cloudera.org:8080/10471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie3b923021ebce5192d2d64784e7ddb952ba82bc3 Gerrit-Change-Number: 10471 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7016: Implement ALTER DATABASE SET OWNER
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/10471 )
Change subject: IMPALA-7016: Implement ALTER DATABASE SET OWNER
..
Patch Set 5:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/10471/3/common/thrift/JniCatalog.thrift
File common/thrift/JniCatalog.thrift:
http://gerrit.cloudera.org:8080/#/c/10471/3/common/thrift/JniCatalog.thrift@55
PS3, Line 55: COMMENT_ON,
> need comma
Done
http://gerrit.cloudera.org:8080/#/c/10471/3/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:
http://gerrit.cloudera.org:8080/#/c/10471/3/fe/src/main/cup/sql-parser.cup@1030
PS3, Line 1030: comment_on_stmt ::=
> Can you add a short comment as to why USER cannot be a keyword?
Done
http://gerrit.cloudera.org:8080/#/c/10471/3/fe/src/main/java/org/apache/impala/analysis/OwnerType.java
File fe/src/main/java/org/apache/impala/analysis/OwnerType.java:
http://gerrit.cloudera.org:8080/#/c/10471/3/fe/src/main/java/org/apache/impala/analysis/OwnerType.java@23
PS3, Line 23:
> nit: Can the name be changed since it's not just type but includes the name
I'll change it to Owner.
http://gerrit.cloudera.org:8080/#/c/10471/3/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
File fe/src/test/java/org/apache/impala/analysis/ParserTest.java:
http://gerrit.cloudera.org:8080/#/c/10471/3/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@3757
PS3, Line 3757: ParserError("ALTER DATABASE SET OWNER foo");
> Add test for no parms after OWNER.
Done
--
To view, visit http://gerrit.cloudera.org:8080/10471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3b923021ebce5192d2d64784e7ddb952ba82bc3
Gerrit-Change-Number: 10471
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Adam Holley
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Wed, 06 Jun 2018 01:02:34 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6812: Fix flaky Kudu scan tests
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10503 ) Change subject: IMPALA-6812: Fix flaky Kudu scan tests .. Patch Set 1: -Code-Review We have the kudu_latest_observed_timestamp logic in Impala that stores the timestamp in the user session after a DML operation. I believe in all the cases we've seen the queries should be part of the same user session (otherwise there are other consistency issues that we'd see). -- To view, visit http://gerrit.cloudera.org:8080/10503 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I70df84f2cbc663107f2ad029565d3c15bdfbd47c Gerrit-Change-Number: 10503 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 06 Jun 2018 00:59:01 + Gerrit-HasComments: No
[Impala-ASF-CR] Fix Indents from IMPALA-4970
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10560 ) Change subject: Fix Indents from IMPALA-4970 .. Patch Set 2: Pretty sure this was an OOM due to https://issues.apache.org/jira/browse/IMPALA-7008 (the patchset wasn't rebased onto a commit with the fix) -- To view, visit http://gerrit.cloudera.org:8080/10560 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4ea53e0780cca9f791808c2a73c7f72ba27c2159 Gerrit-Change-Number: 10560 Gerrit-PatchSet: 2 Gerrit-Owner: Rahul Shivu Mahadev Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Rahul Shivu Mahadev Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 06 Jun 2018 00:43:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7078: Part 1: improve memory consumption of wide Avro scans
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10550 ) Change subject: IMPALA-7078: Part 1: improve memory consumption of wide Avro scans .. Patch Set 11: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10550 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iebd2600b4784fd19696c9b92eefb7d7e9db0c80b Gerrit-Change-Number: 10550 Gerrit-PatchSet: 11 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 06 Jun 2018 00:33:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7078: Part 2: reduce queue size based on num scanner threads
Hello Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10573 to look at the new patch set (#4). Change subject: IMPALA-7078: Part 2: reduce queue size based on num_scanner_threads .. IMPALA-7078: Part 2: reduce queue size based on num_scanner_threads This gives a tighter bound on memory consumption when running with a lower num_scanner_threads value. With IMPALA-7096 we'll revisit the approach to reliably avoid OOM. Cap the maximum row batch queue size at 5 * max_num_scanner_threads_ so that we guarantee lower amounts of memory in the row batch queue when num_scanner_threads is set, rather than just achieving it statistically because of the producer running slower relative to consumer. It does not reduce the default significantly on typical server configurations that would have 24+ cores except under high concurrency or low memory environments where the number of scanner threads is limited. We should evaluate reducing the default further or otherwise better controlling memory consumption in a follow-up, based on experiments. Testing: Tested along with Part 1. Change-Id: I8c0bd14e6bcd9fc1655e344a5307ea0eb4600e8b --- M be/src/exec/hdfs-scan-node.cc 1 file changed, 19 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/10573/4 -- To view, visit http://gerrit.cloudera.org:8080/10573 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8c0bd14e6bcd9fc1655e344a5307ea0eb4600e8b Gerrit-Change-Number: 10573 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7078: Part 1: improve memory consumption of wide Avro scans
Hello Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10550 to look at the new patch set (#11). Change subject: IMPALA-7078: Part 1: improve memory consumption of wide Avro scans .. IMPALA-7078: Part 1: improve memory consumption of wide Avro scans Revert to the pre-IMPALA-3905 algorithm for deciding when to return a batch from an Avro scan. The post-IMPALA-3905 algorithm is bad for wide tables where there are only a small number of rows per Avro block. Optimise memory transfer for selective scans - don't attach unused decompression buffers to the output batch. Combined with the previous change, this dramatically reduces the amount of memory transferred out of scanner threads for selective scans of wide tables. Includes some observability improvements including additional counters that will help diagnose issues like this more easily: * Add counters to give some insight into row batch queue. Here's an excerpt: - RowBatchBytesEnqueued: 20.89 MB (21903380) - RowBatchQueueCapacity: 5 (5) - RowBatchQueueGetWaitTime: 59.187ms - RowBatchQueuePeakMemoryUsage: 8.85 MB (9279347) - RowBatchQueuePutWaitTime: 0.000ns - RowBatchesEnqueued: 6 (6) * Don't create AverageScannerThreadConcurrency for MT scan node where it's not actually used. * Track the row batch queue memory consumption against a sub-tracker HDFS_SCAN_NODE (id=2): Reservation=48.00 MB OtherMemory=588.00 KB Total=48.57 MB Peak=48.62 MB Queued Batches: Total=588.00 KB Peak=637.00 KB Ran the repro in the JIRA. Memory consumption was reduced from ~500MB to ~220MB on my system. Testing: * Ran stress test for an hour on uncompressed and 3 hours on snappy-compressed avro. * Debug exhaustive tests passed. * ASAN core tests passed. Perf: - Parquet TPC-H scale factor 60 on one impalad showed no change in perf - Avro/Snappy scale factor 20 showed a small improvement: +--+-+-++++ | Workload | File Format | Avg (s) | Delta(Avg) | GeoMean(s) | Delta(GeoMean) | +--+-+-++++ | TPCH(20) | avro / snap / block | 9.86| -2.23% | 7.83 | -2.37% | +--+-+-++++ +--+--+--++-++---++-+---+ | Workload | Query| File Format | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%) | Base StdDev(%) | Num Clients | Iters | +--+--+--++-++---++-+---+ | TPCH(20) | TPCH-Q6 | avro / block / block | 5.59 | 5.17| +8.10% | 0.75% | 0.71%| 1 | 30| | TPCH(20) | TPCH-Q14 | avro / block / block | 6.31 | 5.89| +7.21% | 0.68% | 0.74%| 1 | 30| | TPCH(20) | TPCH-Q15 | avro / block / block | 11.32 | 10.64 | +6.41% | 0.37% | 0.46%| 1 | 30| | TPCH(20) | TPCH-Q12 | avro / block / block | 8.57 | 8.14| +5.23% | 0.67% | 0.81%| 1 | 30| | TPCH(20) | TPCH-Q13 | avro / block / block | 6.72 | 6.54| +2.72% | 0.77% | 0.73%| 1 | 30| | TPCH(20) | TPCH-Q4 | avro / block / block | 11.76 | 11.61 | +1.32% | 0.60% | 0.61%| 1 | 30| | TPCH(20) | TPCH-Q7 | avro / block / block | 14.43 | 14.26 | +1.21% | 1.14% | 0.35%| 1 | 30| | TPCH(20) | TPCH-Q21 | avro / block / block | 34.12 | 34.25 | -0.36% | 0.27% | 0.24%| 1 | 30| | TPCH(20) | TPCH-Q20 | avro / block / block | 8.49 | 8.52| -0.38% | 0.45% | 0.54%| 1 | 30| | TPCH(20) | TPCH-Q1 | avro / block / block | 6.99 | 7.02| -0.38% | 0.96% | 0.65%| 1 | 30| | TPCH(20) | TPCH-Q22 | avro / block / block | 2.44 | 2.47| -1.09% | 1.81% | 1.47%| 1 | 30| | TPCH(20) | TPCH-Q11 | avro / block / block | 1.99 | 2.02| -1.57% | 1.95% | 1.90%| 1 | 30| | TPCH(20) | TPCH-Q17 | avro / block / block | 13.57 | 13.79 | -1.63% | 1.53% | 1.31%| 1 | 30| | TPCH(20) | TPCH-Q18 | avro / block / block | 21.93 | 22.31 | -1.72% | 0.31% | 0.34%| 1 | 30| | TPCH(20) | TPCH-Q8 | avro / block / block | 9.05 | 9.31| -2.81% | 0.85% | 0.72%| 1 | 30| | TPCH(20) | TPCH-Q19 | avro / block / block | 7.20 | 7.41| -2.91% | 0.72% | 0.52%| 1 | 30
[Impala-ASF-CR] IMPALA-7078: Part 2: reduce queue size based on num scanner threads
Hello Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10573 to look at the new patch set (#3). Change subject: IMPALA-7078: Part 2: reduce queue size based on num_scanner_threads .. IMPALA-7078: Part 2: reduce queue size based on num_scanner_threads This gives a tighter bound on memory consumption when running with a lower num_scanner_threads value. With IMPALA-7096 we'll revisit the approach to reliably avoid OOM. Cap the maximum row batch queue size at 5 * max_num_scanner_threads_ so that we guarantee lower amounts of memory in the row batch queue when num_scanner_threads is set, rather than just achieving it statistically because of the producer running slower relative to consumer. It does not reduce the default significantly on typical server configurations that would have 24+ cores except under high concurrency or low memory environments where the number of scanner threads is limited. We should evaluate reducing the default further or otherwise better controlling memory consumption in a follow-up, based on experiments. Testing: Tested along with Part 1. Change-Id: I8c0bd14e6bcd9fc1655e344a5307ea0eb4600e8b --- M be/src/exec/hdfs-scan-node.cc 1 file changed, 19 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/10573/3 -- To view, visit http://gerrit.cloudera.org:8080/10573 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8c0bd14e6bcd9fc1655e344a5307ea0eb4600e8b Gerrit-Change-Number: 10573 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht
[Impala-ASF-CR] IMPALA-7078: Part 1: improve memory consumption of wide Avro scans
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10550 ) Change subject: IMPALA-7078: Part 1: improve memory consumption of wide Avro scans .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/10550/9/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/10550/9/be/src/exec/hdfs-scan-node.cc@a171 PS9, Line 171: Oops! -- To view, visit http://gerrit.cloudera.org:8080/10550 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iebd2600b4784fd19696c9b92eefb7d7e9db0c80b Gerrit-Change-Number: 10550 Gerrit-PatchSet: 9 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 06 Jun 2018 00:31:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7078: Part 1: improve memory consumption of wide Avro scans
Hello Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10550 to look at the new patch set (#10). Change subject: IMPALA-7078: Part 1: improve memory consumption of wide Avro scans .. IMPALA-7078: Part 1: improve memory consumption of wide Avro scans Revert to the pre-IMPALA-3905 algorithm for deciding when to return a batch from an Avro scan. The post-IMPALA-3905 algorithm is bad for wide tables where there are only a small number of rows per Avro block. Optimise memory transfer for selective scans - don't attach unused decompression buffers to the output batch. Combined with the previous change, this dramatically reduces the amount of memory transferred out of scanner threads for selective scans of wide tables. Includes some observability improvements including additional counters that will help diagnose issues like this more easily: * Add counters to give some insight into row batch queue. Here's an excerpt: - RowBatchBytesEnqueued: 20.89 MB (21903380) - RowBatchQueueCapacity: 5 (5) - RowBatchQueueGetWaitTime: 59.187ms - RowBatchQueuePeakMemoryUsage: 8.85 MB (9279347) - RowBatchQueuePutWaitTime: 0.000ns - RowBatchesEnqueued: 6 (6) * Don't create AverageScannerThreadConcurrency for MT scan node where it's not actually used. * Track the row batch queue memory consumption against a sub-tracker HDFS_SCAN_NODE (id=2): Reservation=48.00 MB OtherMemory=588.00 KB Total=48.57 MB Peak=48.62 MB Queued Batches: Total=588.00 KB Peak=637.00 KB Ran the repro in the JIRA. Memory consumption was reduced from ~500MB to ~220MB on my system. Testing: * Ran stress test for an hour on uncompressed and 3 hours on snappy-compressed avro. * Debug exhaustive tests passed. * ASAN core tests passed. Perf: - Parquet TPC-H scale factor 60 on one impalad showed no change in perf - Avro/Snappy scale factor 20 showed a small improvement: +--+-+-++++ | Workload | File Format | Avg (s) | Delta(Avg) | GeoMean(s) | Delta(GeoMean) | +--+-+-++++ | TPCH(20) | avro / snap / block | 9.86| -2.23% | 7.83 | -2.37% | +--+-+-++++ +--+--+--++-++---++-+---+ | Workload | Query| File Format | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%) | Base StdDev(%) | Num Clients | Iters | +--+--+--++-++---++-+---+ | TPCH(20) | TPCH-Q6 | avro / block / block | 5.59 | 5.17| +8.10% | 0.75% | 0.71%| 1 | 30| | TPCH(20) | TPCH-Q14 | avro / block / block | 6.31 | 5.89| +7.21% | 0.68% | 0.74%| 1 | 30| | TPCH(20) | TPCH-Q15 | avro / block / block | 11.32 | 10.64 | +6.41% | 0.37% | 0.46%| 1 | 30| | TPCH(20) | TPCH-Q12 | avro / block / block | 8.57 | 8.14| +5.23% | 0.67% | 0.81%| 1 | 30| | TPCH(20) | TPCH-Q13 | avro / block / block | 6.72 | 6.54| +2.72% | 0.77% | 0.73%| 1 | 30| | TPCH(20) | TPCH-Q4 | avro / block / block | 11.76 | 11.61 | +1.32% | 0.60% | 0.61%| 1 | 30| | TPCH(20) | TPCH-Q7 | avro / block / block | 14.43 | 14.26 | +1.21% | 1.14% | 0.35%| 1 | 30| | TPCH(20) | TPCH-Q21 | avro / block / block | 34.12 | 34.25 | -0.36% | 0.27% | 0.24%| 1 | 30| | TPCH(20) | TPCH-Q20 | avro / block / block | 8.49 | 8.52| -0.38% | 0.45% | 0.54%| 1 | 30| | TPCH(20) | TPCH-Q1 | avro / block / block | 6.99 | 7.02| -0.38% | 0.96% | 0.65%| 1 | 30| | TPCH(20) | TPCH-Q22 | avro / block / block | 2.44 | 2.47| -1.09% | 1.81% | 1.47%| 1 | 30| | TPCH(20) | TPCH-Q11 | avro / block / block | 1.99 | 2.02| -1.57% | 1.95% | 1.90%| 1 | 30| | TPCH(20) | TPCH-Q17 | avro / block / block | 13.57 | 13.79 | -1.63% | 1.53% | 1.31%| 1 | 30| | TPCH(20) | TPCH-Q18 | avro / block / block | 21.93 | 22.31 | -1.72% | 0.31% | 0.34%| 1 | 30| | TPCH(20) | TPCH-Q8 | avro / block / block | 9.05 | 9.31| -2.81% | 0.85% | 0.72%| 1 | 30| | TPCH(20) | TPCH-Q19 | avro / block / block | 7.20 | 7.41| -2.91% | 0.72% | 0.52%| 1 | 30
[Impala-ASF-CR] IMPALA-7078: Part 2: reduce queue size based on num scanner threads
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10573 ) Change subject: IMPALA-7078: Part 2: reduce queue size based on num_scanner_threads .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/10573/2/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/10573/2/be/src/exec/hdfs-scan-node.cc@204 PS2, Line 204: f (file_descs_.empty() || progress_.done()) return Status::OK(); > is this somehow related to this change? why do this? This was an error in separating out this from the previous patchset. The previous patchset erroneously deleted this line. -- To view, visit http://gerrit.cloudera.org:8080/10573 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c0bd14e6bcd9fc1655e344a5307ea0eb4600e8b Gerrit-Change-Number: 10573 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 06 Jun 2018 00:31:25 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7078: Part 1: improve memory consumption of wide Avro scans
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10550 ) Change subject: IMPALA-7078: Part 1: improve memory consumption of wide Avro scans .. Patch Set 9: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10550 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iebd2600b4784fd19696c9b92eefb7d7e9db0c80b Gerrit-Change-Number: 10550 Gerrit-PatchSet: 9 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 06 Jun 2018 00:26:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7078: Part 1: improve memory consumption of wide Avro scans
Hello Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10550 to look at the new patch set (#8). Change subject: IMPALA-7078: Part 1: improve memory consumption of wide Avro scans .. IMPALA-7078: Part 1: improve memory consumption of wide Avro scans Revert to the pre-IMPALA-3905 algorithm for deciding when to return a batch from an Avro scan. The post-IMPALA-3905 algorithm is bad for wide tables where there are only a small number of rows per Avro block. Optimise memory transfer for selective scans - don't attach unused decompression buffers to the output batch. Combined with the previous change, this dramatically reduces the amount of memory transferred out of scanner threads for selective scans of wide tables. Includes some observability improvements including additional counters that will help diagnose issues like this more easily: * Add counters to give some insight into row batch queue. Here's an excerpt: - RowBatchBytesEnqueued: 20.89 MB (21903380) - RowBatchQueueCapacity: 5 (5) - RowBatchQueueGetWaitTime: 59.187ms - RowBatchQueuePeakMemoryUsage: 8.85 MB (9279347) - RowBatchQueuePutWaitTime: 0.000ns - RowBatchesEnqueued: 6 (6) * Don't create AverageScannerThreadConcurrency for MT scan node where it's not actually used. * Track the row batch queue memory consumption against a sub-tracker HDFS_SCAN_NODE (id=2): Reservation=48.00 MB OtherMemory=588.00 KB Total=48.57 MB Peak=48.62 MB Queued Batches: Total=588.00 KB Peak=637.00 KB Ran the repro in the JIRA. Memory consumption was reduced from ~500MB to ~220MB on my system. Testing: * Ran stress test for an hour on uncompressed and 3 hours on snappy-compressed avro. * Debug exhaustive tests passed. * ASAN core tests passed. Perf: - Parquet TPC-H scale factor 60 on one impalad showed no change in perf - Avro/Snappy scale factor 20 showed a small improvement: +--+-+-++++ | Workload | File Format | Avg (s) | Delta(Avg) | GeoMean(s) | Delta(GeoMean) | +--+-+-++++ | TPCH(20) | avro / snap / block | 9.86| -2.23% | 7.83 | -2.37% | +--+-+-++++ +--+--+--++-++---++-+---+ | Workload | Query| File Format | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%) | Base StdDev(%) | Num Clients | Iters | +--+--+--++-++---++-+---+ | TPCH(20) | TPCH-Q6 | avro / block / block | 5.59 | 5.17| +8.10% | 0.75% | 0.71%| 1 | 30| | TPCH(20) | TPCH-Q14 | avro / block / block | 6.31 | 5.89| +7.21% | 0.68% | 0.74%| 1 | 30| | TPCH(20) | TPCH-Q15 | avro / block / block | 11.32 | 10.64 | +6.41% | 0.37% | 0.46%| 1 | 30| | TPCH(20) | TPCH-Q12 | avro / block / block | 8.57 | 8.14| +5.23% | 0.67% | 0.81%| 1 | 30| | TPCH(20) | TPCH-Q13 | avro / block / block | 6.72 | 6.54| +2.72% | 0.77% | 0.73%| 1 | 30| | TPCH(20) | TPCH-Q4 | avro / block / block | 11.76 | 11.61 | +1.32% | 0.60% | 0.61%| 1 | 30| | TPCH(20) | TPCH-Q7 | avro / block / block | 14.43 | 14.26 | +1.21% | 1.14% | 0.35%| 1 | 30| | TPCH(20) | TPCH-Q21 | avro / block / block | 34.12 | 34.25 | -0.36% | 0.27% | 0.24%| 1 | 30| | TPCH(20) | TPCH-Q20 | avro / block / block | 8.49 | 8.52| -0.38% | 0.45% | 0.54%| 1 | 30| | TPCH(20) | TPCH-Q1 | avro / block / block | 6.99 | 7.02| -0.38% | 0.96% | 0.65%| 1 | 30| | TPCH(20) | TPCH-Q22 | avro / block / block | 2.44 | 2.47| -1.09% | 1.81% | 1.47%| 1 | 30| | TPCH(20) | TPCH-Q11 | avro / block / block | 1.99 | 2.02| -1.57% | 1.95% | 1.90%| 1 | 30| | TPCH(20) | TPCH-Q17 | avro / block / block | 13.57 | 13.79 | -1.63% | 1.53% | 1.31%| 1 | 30| | TPCH(20) | TPCH-Q18 | avro / block / block | 21.93 | 22.31 | -1.72% | 0.31% | 0.34%| 1 | 30| | TPCH(20) | TPCH-Q8 | avro / block / block | 9.05 | 9.31| -2.81% | 0.85% | 0.72%| 1 | 30| | TPCH(20) | TPCH-Q19 | avro / block / block | 7.20 | 7.41| -2.91% | 0.72% | 0.52%| 1 | 30
[Impala-ASF-CR] IMPALA-7078: Part 1: improve memory consumption of wide Avro scans
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/10550 )
Change subject: IMPALA-7078: Part 1: improve memory consumption of wide Avro
scans
..
Patch Set 7:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/exec/hdfs-avro-scanner.cc
File be/src/exec/hdfs-avro-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/exec/hdfs-avro-scanner.cc@498
PS7, Line 498: starts off with AtCapacity() == true.
> under which case do we get here with row_batch already AtCapacity()? Not as
We can get there if the tuple buffer is > 8mb, e.g. for extremely wide rows.
http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/exec/hdfs-avro-scanner.cc@498
PS7, Line 498: mke
> make
Done
http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/runtime/mem-tracker.h
File be/src/runtime/mem-tracker.h:
http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/runtime/mem-tracker.h@238
PS7, Line 238: Must not be used if
> do we have cases that transfer in this case? if so, what's the recommended
I think the data stream receiver might have this case. For now we do a
Release() and Consume(). Maybe, ideally, we should do a TryTransfer() or
something like that?
http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:
http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/runtime/mem-tracker.cc@236
PS7, Line 236: && all_trackers_[ancestor_idx - 1] ==
dst->all_trackers_[dst_ancestor_idx - 1]) {
> should we dcheck that the limits are not set?
These MemTrackers can have limits, it's the ones below the common ancestors
that are not allowed to have limits. ConsumeLocal() enforces this.
--
To view, visit http://gerrit.cloudera.org:8080/10550
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebd2600b4784fd19696c9b92eefb7d7e9db0c80b
Gerrit-Change-Number: 10550
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 06 Jun 2018 00:26:22 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6956: deflake and logging for query expiration test
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10602 ) Change subject: IMPALA-6956: deflake and logging for query_expiration test .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2603/ -- To view, visit http://gerrit.cloudera.org:8080/10602 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I01a8762d28ad920b9ec8a0b1b82469618c66768f Gerrit-Change-Number: 10602 Gerrit-PatchSet: 2 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 05 Jun 2018 23:52:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7078: Part 2: reduce queue size based on num scanner threads
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10573 ) Change subject: IMPALA-7078: Part 2: reduce queue size based on num_scanner_threads .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/10573/2/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/10573/2/be/src/exec/hdfs-scan-node.cc@204 PS2, Line 204: f (file_descs_.empty() || progress_.done()) return Status::OK(); is this somehow related to this change? why do this? -- To view, visit http://gerrit.cloudera.org:8080/10573 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c0bd14e6bcd9fc1655e344a5307ea0eb4600e8b Gerrit-Change-Number: 10573 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Comment-Date: Tue, 05 Jun 2018 23:48:37 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6956: deflake and logging for query expiration test
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10602 ) Change subject: IMPALA-6956: deflake and logging for query_expiration test .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10602 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I01a8762d28ad920b9ec8a0b1b82469618c66768f Gerrit-Change-Number: 10602 Gerrit-PatchSet: 2 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 05 Jun 2018 23:48:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10601 ) Change subject: IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang .. Patch Set 6: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c147550f86d81b818ecbdd34cf2919ced7ff8c5 Gerrit-Change-Number: 10601 Gerrit-PatchSet: 6 Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jun 2018 23:47:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10601 ) Change subject: IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang .. IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang If we hit EOS, we'll wait for all the backends to report status (to try to get a complete profile). But if the query is closed after this point, then we can get stuck waiting since once the query is closed, ImpalaServer won't know about this coordinator and so it will stop forwarding on the ReportStatus RPCs. The real fix for this is IMPALA-6984, but in the mean time, add another special case for this JIRA (see the other TODO IMPALA-6984 in coordinator.cc). The cancellation test only finds this race once in a while (several hours) indirectly in a COMPUTE STATS query because the ChildQueryExecutor will do a CloseOperation() while the execution thread is inside Fetch(). To make this more reproducible, modify the cancellation test to allow the close and fetch rpcs to execute concurrently (don't join the test's fetch thread until after close). This makes the race reproducible in a few iterations and a few minutes. Testing: - Loop test_cancellation.py Change-Id: I7c147550f86d81b818ecbdd34cf2919ced7ff8c5 Reviewed-on: http://gerrit.cloudera.org:8080/10601 Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins --- M be/src/runtime/coordinator.cc M tests/query_test/test_cancellation.py 2 files changed, 47 insertions(+), 9 deletions(-) Approvals: Tim Armstrong: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I7c147550f86d81b818ecbdd34cf2919ced7ff8c5 Gerrit-Change-Number: 10601 Gerrit-PatchSet: 7 Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6956: deflake and logging for query expiration test
Hello Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10602 to look at the new patch set (#2). Change subject: IMPALA-6956: deflake and logging for query_expiration test .. IMPALA-6956: deflake and logging for query_expiration test There was a recent flake where the number of in-flight queries that were executing differed from the expected number. The reason for the false negative is that one of the queries expired before the check for in-flight queries: it took too long to issue the queries. This change modifies the timeout for the expired query to not expire so quickly. Additional logging is added to the check for in-flight queries so that we can distinguish the case of too few queries vs. queries that have the wrong state. Change-Id: I01a8762d28ad920b9ec8a0b1b82469618c66768f --- M tests/custom_cluster/test_query_expiration.py 1 file changed, 9 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/10602/2 -- To view, visit http://gerrit.cloudera.org:8080/10602 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I01a8762d28ad920b9ec8a0b1b82469618c66768f Gerrit-Change-Number: 10602 Gerrit-PatchSet: 2 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6956: deflake and logging for query expiration test
Vuk Ercegovac has posted comments on this change. (
http://gerrit.cloudera.org:8080/10602 )
Change subject: IMPALA-6956: deflake and logging for query_expiration test
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10602/1/tests/custom_cluster/test_query_expiration.py
File tests/custom_cluster/test_query_expiration.py:
http://gerrit.cloudera.org:8080/#/c/10602/1/tests/custom_cluster/test_query_expiration.py@58
PS1, Line 58: client.execute("SET EXEC_TIME_LIMIT_S=3")
> The problem I saw was that it just took longer than assumed to get these fo
ah, you're right about the second query. there were two tests, one failed with
one query off and other with both these queries off.
updated the other query as well.
--
To view, visit http://gerrit.cloudera.org:8080/10602
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I01a8762d28ad920b9ec8a0b1b82469618c66768f
Gerrit-Change-Number: 10602
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Tue, 05 Jun 2018 23:42:44 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10601 ) Change subject: IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c147550f86d81b818ecbdd34cf2919ced7ff8c5 Gerrit-Change-Number: 10601 Gerrit-PatchSet: 6 Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jun 2018 23:41:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7106: Print rewritten SQL correctly when log trace is enabled
Fredy Wijaya has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/10571 ) Change subject: IMPALA-7106: Print rewritten SQL correctly when log trace is enabled .. IMPALA-7106: Print rewritten SQL correctly when log trace is enabled toSql() method is used to print SQL string that is close to the original SQL string, which is something that users want to see. When debugging issues related to SQL rewrites, it can be very useful to be able to print/get the SQL string that is being rewritten. This patch adds a new method toRewrittenSql() to get the rewritten SQL string. The LOG.trace statement that prints the rewritten SQL is also updated to use toRewrittenSql(). Testing: - Added FE test for the rewritten SQL string - Ran all FE tests Change-Id: Iab58b0cc865135d261dd4a7f72be130f2e7bde53 --- M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java M fe/src/main/java/org/apache/impala/analysis/FromClause.java M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/main/java/org/apache/impala/analysis/StatementBase.java M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java M fe/src/main/java/org/apache/impala/analysis/TableRef.java M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java 9 files changed, 145 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/10571/7 -- To view, visit http://gerrit.cloudera.org:8080/10571 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iab58b0cc865135d261dd4a7f72be130f2e7bde53 Gerrit-Change-Number: 10571 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6035: Add query options to limit thread reservation
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10365 ) Change subject: IMPALA-6035: Add query options to limit thread reservation .. Patch Set 9: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10365 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5b5bbbdad5cd6b24442eb6c99a4d38c2ad710007 Gerrit-Change-Number: 10365 Gerrit-PatchSet: 9 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jun 2018 23:40:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7106: Print rewritten SQL correctly when log trace is enabled
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/10571 ) Change subject: IMPALA-7106: Print rewritten SQL correctly when log trace is enabled .. Patch Set 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/10571/3/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/10571/3/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@972 PS3, Line 972: > I tried "SELECT * FROM (SELECT (CASE WHEN TRUE THEN 1 ELSE id END) FROM fun I forgot to update the InlineRefView. Done. http://gerrit.cloudera.org:8080/#/c/10571/5/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java File fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java: http://gerrit.cloudera.org:8080/#/c/10571/5/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java@117 PS5, Line 117: stmt.sqlString_ = null; > I don't quite understand why you chose to restore the sql string this way. I did try removing this line and it didn't work. It also broke other tests. -- To view, visit http://gerrit.cloudera.org:8080/10571 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iab58b0cc865135d261dd4a7f72be130f2e7bde53 Gerrit-Change-Number: 10571 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 05 Jun 2018 23:40:10 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7078: Part 1: improve memory consumption of wide Avro scans
Dan Hecht has posted comments on this change. (
http://gerrit.cloudera.org:8080/10550 )
Change subject: IMPALA-7078: Part 1: improve memory consumption of wide Avro
scans
..
Patch Set 7: Code-Review+2
(4 comments)
http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/exec/hdfs-avro-scanner.cc
File be/src/exec/hdfs-avro-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/exec/hdfs-avro-scanner.cc@498
PS7, Line 498: starts off with AtCapacity() == true.
under which case do we get here with row_batch already AtCapacity()? Not asking
to restructure now, but why is it structured that way?
http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/exec/hdfs-avro-scanner.cc@498
PS7, Line 498: mke
make
http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/runtime/mem-tracker.h
File be/src/runtime/mem-tracker.h:
http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/runtime/mem-tracker.h@238
PS7, Line 238: Must not be used if
do we have cases that transfer in this case? if so, what's the recommended
pattern? (Release() and Consume()?)
http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:
http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/runtime/mem-tracker.cc@236
PS7, Line 236: && all_trackers_[ancestor_idx - 1] ==
dst->all_trackers_[dst_ancestor_idx - 1]) {
should we dcheck that the limits are not set?
--
To view, visit http://gerrit.cloudera.org:8080/10550
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebd2600b4784fd19696c9b92eefb7d7e9db0c80b
Gerrit-Change-Number: 10550
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Tue, 05 Jun 2018 23:17:08 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7050: [DOCS] Document the max serialized incremental stat size setting
Hello Balazs Jeszenszky, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10457 to look at the new patch set (#4). Change subject: IMPALA-7050: [DOCS] Document the max serialized incremental stat size setting .. IMPALA-7050: [DOCS] Document the max serialized incremental stat size setting Change-Id: Ifa80325f0008d42a9cc8178e7c144fc2b49d7d4e --- M docs/topics/impala_perf_stats.xml 1 file changed, 35 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/10457/4 -- To view, visit http://gerrit.cloudera.org:8080/10457 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifa80325f0008d42a9cc8178e7c144fc2b49d7d4e Gerrit-Change-Number: 10457 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7050: [DOCS] Document the max serialized incremental stat size setting
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/10457 ) Change subject: IMPALA-7050: [DOCS] Document the max serialized incremental stat size setting .. Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/10457/3/docs/topics/impala_perf_stats.xml File docs/topics/impala_perf_stats.xml: http://gerrit.cloudera.org:8080/#/c/10457/3/docs/topics/impala_perf_stats.xml@709 PS3, Line 709: When you need to run COMPUTE INCREMENTAL STATS on : very large tables, you can use the configuration setting : inc_stats_size_limit_bytes to reduce the load on : the catalog server. > turn into passive? Attempted. http://gerrit.cloudera.org:8080/#/c/10457/3/docs/topics/impala_perf_stats.xml@714 PS3, Line 714: This limit is set as a safety check, to prevent the JVM from hitting : a maximum array limit of 1 GB or runs out of memory. > the limit is 2GB with suggested JVM versions. Readers would need to underst Done http://gerrit.cloudera.org:8080/#/c/10457/3/docs/topics/impala_perf_stats.xml@724 PS3, Line 724: To change the inc_stats_size_limit_bytes value > This is a startup option for the impala daemons and the catalogd, not a cli Do I have to restart both impalad and catalogd after changing the option? http://gerrit.cloudera.org:8080/#/c/10457/3/docs/topics/impala_perf_stats.xml@733 PS3, Line 733: spike in heap usage > as well as crashes. Done http://gerrit.cloudera.org:8080/#/c/10457/3/docs/topics/impala_perf_stats.xml@736 PS3, Line 736: : Setting inc_stats_size_limit_bytes to a big value, : such as 1 GB or more, can result in a spike in heap usage. : > duplicate of above? Done http://gerrit.cloudera.org:8080/#/c/10457/3/docs/topics/impala_shell_options.xml File docs/topics/impala_shell_options.xml: http://gerrit.cloudera.org:8080/#/c/10457/3/docs/topics/impala_shell_options.xml@570 PS3, Line 570: --inc_stats_size_limit_bytes > this is an impalad-side option Removed from this doc. -- To view, visit http://gerrit.cloudera.org:8080/10457 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifa80325f0008d42a9cc8178e7c144fc2b49d7d4e Gerrit-Change-Number: 10457 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 05 Jun 2018 22:56:10 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7105. Ensure fe tests pass when running standalone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10570 ) Change subject: IMPALA-7105. Ensure fe tests pass when running standalone .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2602/ -- To view, visit http://gerrit.cloudera.org:8080/10570 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I85286ba44f03526b487212a8ddadf71a9f427555 Gerrit-Change-Number: 10570 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 05 Jun 2018 22:36:21 + Gerrit-HasComments: No
[Impala-ASF-CR] Add missing namespace qualifiers
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10608 ) Change subject: Add missing namespace qualifiers .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10608 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ef8f5c8f05100418dec90c2cd2e2b3dcd827c27 Gerrit-Change-Number: 10608 Gerrit-PatchSet: 1 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jun 2018 22:34:08 + Gerrit-HasComments: No
[Impala-ASF-CR] Add missing namespace qualifiers
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10608 ) Change subject: Add missing namespace qualifiers .. Add missing namespace qualifiers As part of an upcoming rebase of the KRPC code, this change adds missing namespace qualifiers in some places. Change-Id: I3ef8f5c8f05100418dec90c2cd2e2b3dcd827c27 Reviewed-on: http://gerrit.cloudera.org:8080/10608 Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins --- M be/src/exec/kudu-util.h M be/src/runtime/descriptors.h 2 files changed, 2 insertions(+), 2 deletions(-) Approvals: Tim Armstrong: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10608 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3ef8f5c8f05100418dec90c2cd2e2b3dcd827c27 Gerrit-Change-Number: 10608 Gerrit-PatchSet: 2 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5552: Add support for authorized proxy groups
Philip Zeyliger has posted comments on this change. (
http://gerrit.cloudera.org:8080/10510 )
Change subject: IMPALA-5552: Add support for authorized proxy groups
..
Patch Set 8:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/10510/8/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/10510/8/be/src/service/impala-server.cc@1389
PS8, Line 1389: Status status = exec_env_->frontend()->GetHadoopGroups(req,
);
Does this code run even if the flag isn't set? If so, someone upgrading but
using a "bad" UserGroupMapping provider will start getting the wrong message.
http://gerrit.cloudera.org:8080/#/c/10510/8/fe/src/main/java/org/apache/impala/service/JniFrontend.java
File fe/src/main/java/org/apache/impala/service/JniFrontend.java:
http://gerrit.cloudera.org:8080/#/c/10510/8/fe/src/main/java/org/apache/impala/service/JniFrontend.java@657
PS8, Line 657: protected static void checkGroupsMappingProvider(Configuration
conf)
We should be doing this at start-up rather than once per request. The issue
isn't so much that it's expensive, but that you want to fail fast on this sort
of configuration error.
If we do it at boot time, we should also log the value of this configuration.
http://gerrit.cloudera.org:8080/#/c/10510/8/fe/src/main/java/org/apache/impala/service/JniFrontend.java@659
PS8, Line 659: GroupMappingServiceProvider provider =
Isn't this just GROUPS?
http://gerrit.cloudera.org:8080/#/c/10510/8/fe/src/main/java/org/apache/impala/service/JniFrontend.java@665
PS8, Line 665: Map recommendations = new HashMap<>();
: recommendations.put(
: ShellBasedUnixGroupsMapping.class.getName(),
: JniBasedUnixGroupsMappingWithFallback.class.getName());
: recommendations.put(
: ShellBasedUnixGroupsNetgroupMapping.class.getName(),
:
JniBasedUnixGroupsNetgroupMappingWithFallback.class.getName());
This could go into the if below.
But, really, just use a longer string. I think the clever map for
recommendations takes away from the point. You can also just use an if/else for
the two cases with the same number of lines.
http://gerrit.cloudera.org:8080/#/c/10510/8/fe/src/main/java/org/apache/impala/service/JniFrontend.java@685
PS8, Line 685: public String checkConfiguration() {
This seems like a good place to check the user group mapping once and for all.
Please look into what it does, of course.
--
To view, visit http://gerrit.cloudera.org:8080/10510
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6953f89c293b06b72f523b11802232133d9d6cbb
Gerrit-Change-Number: 10510
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Greg Rahn
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Sailesh Mukil
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Tue, 05 Jun 2018 22:32:31 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5737: Tighten minicluster memory limit
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10277 ) Change subject: IMPALA-5737: Tighten minicluster memory limit .. Patch Set 7: Code-Review-2 Please don't merge this until I give it the OK, this seems risky. -- To view, visit http://gerrit.cloudera.org:8080/10277 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If8240551e726c6da546a926a1ce3444f41ef87fe Gerrit-Change-Number: 10277 Gerrit-PatchSet: 7 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jun 2018 22:22:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7110. Fix errors from error-prone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10583 ) Change subject: IMPALA-7110. Fix errors from error-prone .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2601/ -- To view, visit http://gerrit.cloudera.org:8080/10583 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie9bfadecb5b92bba4fc7921a6f87f249ed90e771 Gerrit-Change-Number: 10583 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 05 Jun 2018 22:18:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7110. Fix errors from error-prone
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/10583 ) Change subject: IMPALA-7110. Fix errors from error-prone .. Patch Set 2: Code-Review+2 maven profile looks fine. -- To view, visit http://gerrit.cloudera.org:8080/10583 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie9bfadecb5b92bba4fc7921a6f87f249ed90e771 Gerrit-Change-Number: 10583 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 05 Jun 2018 22:16:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7110. Fix errors from error-prone
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/10583 ) Change subject: IMPALA-7110. Fix errors from error-prone .. Patch Set 2: Code-Review+1 (1 comment) Phil, can you +2 this, I'm not too familiar with mvn profiles, rest of it lgtm. http://gerrit.cloudera.org:8080/#/c/10583/1/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java File fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java: http://gerrit.cloudera.org:8080/#/c/10583/1/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java@85 PS1, Line 85: argTypes > argTypes is a List, so the built-in toString actually works. My fix down be ah, nice catch. -- To view, visit http://gerrit.cloudera.org:8080/10583 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie9bfadecb5b92bba4fc7921a6f87f249ed90e771 Gerrit-Change-Number: 10583 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 05 Jun 2018 22:11:57 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7012: Fix NPE when parsing unexpected tokens
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10512 ) Change subject: IMPALA-7012: Fix NPE when parsing unexpected tokens .. IMPALA-7012: Fix NPE when parsing unexpected tokens Currently some token are not added to tokenIdMap in the sql scanner and an NPE will be thrown if such are parsed as unexpected tokens. This patch fixes this bug and adds a case to ParserTest. Change-Id: I9c846fdfb22ba37bfc3b1985b9a044014ab58968 Reviewed-on: http://gerrit.cloudera.org:8080/10512 Reviewed-by: Tianyi Wang Tested-by: Impala Public Jenkins --- M fe/src/main/jflex/sql-scanner.flex M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java 3 files changed, 36 insertions(+), 21 deletions(-) Approvals: Tianyi Wang: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10512 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I9c846fdfb22ba37bfc3b1985b9a044014ab58968 Gerrit-Change-Number: 10512 Gerrit-PatchSet: 4 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang
[Impala-ASF-CR] IMPALA-7012: Fix NPE when parsing unexpected tokens
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10512 ) Change subject: IMPALA-7012: Fix NPE when parsing unexpected tokens .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10512 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c846fdfb22ba37bfc3b1985b9a044014ab58968 Gerrit-Change-Number: 10512 Gerrit-PatchSet: 3 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Comment-Date: Tue, 05 Jun 2018 22:09:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7077. Add a configuration for the max number of partitions to load
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10584 ) Change subject: IMPALA-7077. Add a configuration for the max number of partitions to load .. Patch Set 1: Another idea that also came up was to go with a flag like the one in this change, but base the limit on memory consumed. That would cover more scenarios for banning a table: num partitions, num files, num columns, incremental stats, or some combination. We can obtain the size estimate at the per partition granularity, so once exceeded, back out as in this change. There's still the downside of a static flag setting (less flexibility) but there is some ease-of-use that can be argued as well. -- To view, visit http://gerrit.cloudera.org:8080/10584 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife99a97a891ed14675303ea472abb2932a72cb51 Gerrit-Change-Number: 10584 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 05 Jun 2018 22:07:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5737: Tighten minicluster memory limit
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10277 ) Change subject: IMPALA-5737: Tighten minicluster memory limit .. Patch Set 7: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2598/ -- To view, visit http://gerrit.cloudera.org:8080/10277 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If8240551e726c6da546a926a1ce3444f41ef87fe Gerrit-Change-Number: 10277 Gerrit-PatchSet: 7 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Comment-Date: Tue, 05 Jun 2018 22:06:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7077. Add a configuration for the max number of partitions to load
Philip Zeyliger has posted comments on this change. (
http://gerrit.cloudera.org:8080/10584 )
Change subject: IMPALA-7077. Add a configuration for the max number of
partitions to load
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/10584/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/10584/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2067
PS1, Line 2067: if (curPartitionCount + allHmsPartitionsToAdd.size() >
HdfsTable.MAX_PARTITIONS_PER_TABLE) {
> Yea, we use 90 chars for Java too. Gerrit defaults to 100.
You can re-configure Gerrit for 90 in some config option if that's your thing.
http://gerrit.cloudera.org:8080/#/c/10584/1/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
File fe/src/test/java/org/apache/impala/catalog/CatalogTest.java:
http://gerrit.cloudera.org:8080/#/c/10584/1/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java@523
PS1, Line 523: public void testPartitionLimit() throws CatalogException {
You added the exception handling in "updatePartitionsFromHms()" and in some
alter table path (i.e., two places), but there seems to be only one code path
tested here. Should we test the other one too here, or is it enough that we
test it in the QueryTest?
--
To view, visit http://gerrit.cloudera.org:8080/10584
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife99a97a891ed14675303ea472abb2932a72cb51
Gerrit-Change-Number: 10584
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Tue, 05 Jun 2018 22:06:36 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7077. Add a configuration for the max number of partitions to load
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/10584 )
Change subject: IMPALA-7077. Add a configuration for the max number of
partitions to load
..
Patch Set 1:
(2 comments)
Todd, what are your thoughts on having a "per table" property that can
blacklist that particular table from loading in Impala? For example, something
like
alter table foo set tblproperties ("load-in-impala" = "false")
The issue with having a global max partitions is that
- It is a start up flag (requires a restart)
- Lets say if someone sets it to 100k, we can still have multiple tables with
90k partitions or something like that
- In many cases users would like to blacklist some specific problematic tables
(say with large incremental stats) and having just a partition limit may not
help
Vuk and I were discussing this offline today and I thought I'd bring this up in
this CR. It is unclear if both these flags can co-exist or we should just have
one of them. Thoughts?
http://gerrit.cloudera.org:8080/#/c/10584/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10584/1//COMMIT_MSG@21
PS1, Line 21: worth the complexity to try to
: somehow limit the addition of partitions during a DML
> The problem is that the catalog update happens at the end of the DML, after
I lean towards (2) but bubble up a meaningful error message for the DML query
and not the generic message that we are not able to load the table. I prefer
(2) because (1) leaves the table on HDFS in an inconsistent state.
http://gerrit.cloudera.org:8080/#/c/10584/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/10584/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2067
PS1, Line 2067: if (curPartitionCount + allHmsPartitionsToAdd.size() >
HdfsTable.MAX_PARTITIONS_PER_TABLE) {
> its not explicitly stated for java in the style guide, but given the limits
Yea, we use 90 chars for Java too. Gerrit defaults to 100.
--
To view, visit http://gerrit.cloudera.org:8080/10584
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife99a97a891ed14675303ea472abb2932a72cb51
Gerrit-Change-Number: 10584
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Tue, 05 Jun 2018 22:00:04 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] Fix Indents from IMPALA-4970
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10560 ) Change subject: Fix Indents from IMPALA-4970 .. Patch Set 2: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2595/ -- To view, visit http://gerrit.cloudera.org:8080/10560 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4ea53e0780cca9f791808c2a73c7f72ba27c2159 Gerrit-Change-Number: 10560 Gerrit-PatchSet: 2 Gerrit-Owner: Rahul Shivu Mahadev Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Rahul Shivu Mahadev Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 05 Jun 2018 21:59:57 + Gerrit-HasComments: No
[Impala-ASF-CR] [experimental] Clang Tidy Diff trial balloon
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/9751 ) Change subject: [experimental] Clang Tidy Diff trial balloon .. Patch Set 2: Any updates on this? -- To view, visit http://gerrit.cloudera.org:8080/9751 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2fb6a13400367fd3d12a4738bbb2dfc944466a7 Gerrit-Change-Number: 9751 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jun 2018 21:54:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7106: Print rewritten SQL correctly when log trace is enabled
Tianyi Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/10571 ) Change subject: IMPALA-7106: Print rewritten SQL correctly when log trace is enabled .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/10571/3/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/10571/3/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@972 PS3, Line 972: .toSql() > - FromClause does not extend from StatementBase. I tried "SELECT * FROM (SELECT (CASE WHEN TRUE THEN 1 ELSE id END) FROM functional.alltypes UNION SELECT 1 + 1) v" on toRewrittenSql and it returns the unrewritten query. Isn't it wired to print the rewritten outmost select block but left the inline view original? http://gerrit.cloudera.org:8080/#/c/10571/5/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java File fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java: http://gerrit.cloudera.org:8080/#/c/10571/5/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java@117 PS5, Line 117: stmt.sqlString_ = null; I don't quite understand why you chose to restore the sql string this way. Have you tried removing this line? I don't know what it's for but I expect we don't fundamentally need it. -- To view, visit http://gerrit.cloudera.org:8080/10571 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iab58b0cc865135d261dd4a7f72be130f2e7bde53 Gerrit-Change-Number: 10571 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 05 Jun 2018 21:47:59 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Guard S3 access check with variable
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10607 ) Change subject: Guard S3 access check with variable .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10607 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1df16b2d037afafb7748f52b64732f680883f16c Gerrit-Change-Number: 10607 Gerrit-PatchSet: 2 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jun 2018 21:47:01 + Gerrit-HasComments: No
[Impala-ASF-CR] Guard S3 access check with variable
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10607 ) Change subject: Guard S3 access check with variable .. Guard S3 access check with variable We've recently seen automated build failures where multiple inclusions of impala-config.sh led to the S3 access check being performed multiple times. This would trigger rate limiting on S3's side, which then caused the jobs to fail. This change adds a guard to only perform the check once, even when including impala-config.sh multiple times. To test this I included impala-config.sh multiple times on my local shell and observed that the check was only performed once. Change-Id: I1df16b2d037afafb7748f52b64732f680883f16c Reviewed-on: http://gerrit.cloudera.org:8080/10607 Reviewed-by: Lars Volker Tested-by: Impala Public Jenkins --- M bin/impala-config.sh 1 file changed, 11 insertions(+), 4 deletions(-) Approvals: Lars Volker: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10607 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I1df16b2d037afafb7748f52b64732f680883f16c Gerrit-Change-Number: 10607 Gerrit-PatchSet: 3 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7106: Print rewritten SQL correctly when log trace is enabled
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/10571 ) Change subject: IMPALA-7106: Print rewritten SQL correctly when log trace is enabled .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/10571/5/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/10571/5/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@949 PS5, Line 949: toSqlString(); > I think this method name is confusing. I think we should rename this. Any s The more I think about it, I don't think it's easy to figure out if the expr has been rewritten if not from the call-site. Calling reset() is one way of determining if an expr is rewritten, but it is also possible to clone() the expr so the reset() never gets called and it'll be hard to figure out if the expr is rewritten. I'm also a bit hesitant of adding another state since we already have so many states and we also need to figure out whether or not to clear the state in the reset(). -- To view, visit http://gerrit.cloudera.org:8080/10571 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iab58b0cc865135d261dd4a7f72be130f2e7bde53 Gerrit-Change-Number: 10571 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 05 Jun 2018 21:43:09 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7016: Implement ALTER DATABASE SET OWNER
Adam Holley has posted comments on this change. (
http://gerrit.cloudera.org:8080/10471 )
Change subject: IMPALA-7016: Implement ALTER DATABASE SET OWNER
..
Patch Set 3:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/10471/3/common/thrift/JniCatalog.thrift
File common/thrift/JniCatalog.thrift:
http://gerrit.cloudera.org:8080/#/c/10471/3/common/thrift/JniCatalog.thrift@55
PS3, Line 55: COMMENT_ON
need comma
http://gerrit.cloudera.org:8080/#/c/10471/3/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:
http://gerrit.cloudera.org:8080/#/c/10471/3/fe/src/main/cup/sql-parser.cup@1030
PS3, Line 1030: {:
Can you add a short comment as to why USER cannot be a keyword?
http://gerrit.cloudera.org:8080/#/c/10471/3/fe/src/main/java/org/apache/impala/analysis/OwnerType.java
File fe/src/main/java/org/apache/impala/analysis/OwnerType.java:
http://gerrit.cloudera.org:8080/#/c/10471/3/fe/src/main/java/org/apache/impala/analysis/OwnerType.java@23
PS3, Line 23: public class OwnerType {
nit: Can the name be changed since it's not just type but includes the name?
e.g. just Owner or OwnerContext
http://gerrit.cloudera.org:8080/#/c/10471/3/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
File fe/src/test/java/org/apache/impala/analysis/ParserTest.java:
http://gerrit.cloudera.org:8080/#/c/10471/3/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@3757
PS3, Line 3757: ParserError("ALTER DATABASE SET OWNER ROLE foo");
Add test for no parms after OWNER.
e.g. ALTER DATABASE db SET OWNER
--
To view, visit http://gerrit.cloudera.org:8080/10471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3b923021ebce5192d2d64784e7ddb952ba82bc3
Gerrit-Change-Number: 10471
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Adam Holley
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Tue, 05 Jun 2018 21:30:46 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5552: Add support for authorized proxy groups
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/10510 ) Change subject: IMPALA-5552: Add support for authorized proxy groups .. Patch Set 8: Philip, can you review this and give a +2 if you don't have any concerns? -- To view, visit http://gerrit.cloudera.org:8080/10510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6953f89c293b06b72f523b11802232133d9d6cbb Gerrit-Change-Number: 10510 Gerrit-PatchSet: 8 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 05 Jun 2018 21:11:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7120: remove sonatype repository
Tim Armstrong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10606 ) Change subject: IMPALA-7120: remove sonatype repository .. IMPALA-7120: remove sonatype repository >From what I can tell, we no longer need this because we get java-cup from a different repository. I started a build and we built the frontend ok, so unless I'm missing something, that seems to confirm that it works. Change-Id: I8b7acee041019eecc8f664f2093dd3df72341a7e Reviewed-on: http://gerrit.cloudera.org:8080/10606 Reviewed-by: Philip Zeyliger Tested-by: Tim Armstrong --- M impala-parent/pom.xml 1 file changed, 0 insertions(+), 26 deletions(-) Approvals: Philip Zeyliger: Looks good to me, approved Tim Armstrong: Verified -- To view, visit http://gerrit.cloudera.org:8080/10606 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I8b7acee041019eecc8f664f2093dd3df72341a7e Gerrit-Change-Number: 10606 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7120: remove sonatype repository
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10606 ) Change subject: IMPALA-7120: remove sonatype repository .. Patch Set 2: Verified+1 Yep, it worked after I blew away ~/.m2/repository -- To view, visit http://gerrit.cloudera.org:8080/10606 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8b7acee041019eecc8f664f2093dd3df72341a7e Gerrit-Change-Number: 10606 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jun 2018 21:10:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7106: Print rewritten SQL correctly when log trace is enabled
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/10571 ) Change subject: IMPALA-7106: Print rewritten SQL correctly when log trace is enabled .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/10571/5/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/10571/5/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@949 PS5, Line 949: toSqlString(); > how do we know that the expression has been rewritten? you're controlling t I think this method name is confusing. I think we should rename this. Any suggestion on the name? It's basically a toSql() that doesn't use any SQL string that was previously stored. -- To view, visit http://gerrit.cloudera.org:8080/10571 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iab58b0cc865135d261dd4a7f72be130f2e7bde53 Gerrit-Change-Number: 10571 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 05 Jun 2018 21:07:14 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6917: Implement COMMENT ON TABLE/VIEW
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10478 ) Change subject: IMPALA-6917: Implement COMMENT ON TABLE/VIEW .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I497c17342f79ff7c99931fd8a0ddec0c79303dbf Gerrit-Change-Number: 10478 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 05 Jun 2018 21:03:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7106: Print rewritten SQL correctly when log trace is enabled
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10571 ) Change subject: IMPALA-7106: Print rewritten SQL correctly when log trace is enabled .. Patch Set 5: (1 comment) thanks for the change. this will be a useful step to make it easier to debug/make sense of errors after the rewrite phase. main comment is about guarantees for the new method. http://gerrit.cloudera.org:8080/#/c/10571/5/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/10571/5/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@949 PS5, Line 949: toSqlString(); how do we know that the expression has been rewritten? you're controlling this from one specific call-site, but I'd prefer that this expr know whether its been rewritten and whether this method makes sense to call. same goes for the other impls. we know that the expr is analyzed-- that's a start. is additional state needed to record if rewritten? -- To view, visit http://gerrit.cloudera.org:8080/10571 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iab58b0cc865135d261dd4a7f72be130f2e7bde53 Gerrit-Change-Number: 10571 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 05 Jun 2018 21:02:44 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7120: remove sonatype repository
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10606 ) Change subject: IMPALA-7120: remove sonatype repository .. Patch Set 2: I'll try that also to confirm. -- To view, visit http://gerrit.cloudera.org:8080/10606 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8b7acee041019eecc8f664f2093dd3df72341a7e Gerrit-Change-Number: 10606 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jun 2018 20:55:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7120: remove sonatype repository
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/10606 ) Change subject: IMPALA-7120: remove sonatype repository .. Patch Set 2: Code-Review+2 If this passes the build, it should be good. I hope you tested it after blowing away your ~/.m2/repository so that the test would be clean, but I'm happy to let Jenkins robots try it. -- To view, visit http://gerrit.cloudera.org:8080/10606 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8b7acee041019eecc8f664f2093dd3df72341a7e Gerrit-Change-Number: 10606 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jun 2018 20:49:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7120: remove sonatype repository
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10606 ) Change subject: IMPALA-7120: remove sonatype repository .. Patch Set 2: I looked at the mvn logs and it appears that this change prevented it from contacting oss.sonatype.org -- To view, visit http://gerrit.cloudera.org:8080/10606 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8b7acee041019eecc8f664f2093dd3df72341a7e Gerrit-Change-Number: 10606 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jun 2018 20:47:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6917: Implement COMMENT ON TABLE/VIEW
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/10478 )
Change subject: IMPALA-6917: Implement COMMENT ON TABLE/VIEW
..
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10478/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/10478/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3554
PS5, Line 3554: msTbl.getParameters().put("comment", co
> loadTableMetadata sets the catalog version, so this line can be removed.
Good catch! Done.
--
To view, visit http://gerrit.cloudera.org:8080/10478
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I497c17342f79ff7c99931fd8a0ddec0c79303dbf
Gerrit-Change-Number: 10478
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Tue, 05 Jun 2018 20:46:28 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7100: [DOCS] Consistent memory alloc across executor nodes
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10561 ) Change subject: IMPALA-7100: [DOCS] Consistent memory alloc across executor nodes .. IMPALA-7100: [DOCS] Consistent memory alloc across executor nodes Change-Id: I22926eb6050f9501624d2041f594fe4ef15be73b Reviewed-on: http://gerrit.cloudera.org:8080/10561 Reviewed-by: Balazs Jeszenszky Reviewed-by: Alex Rodoni Tested-by: Impala Public Jenkins --- M docs/topics/impala_prereqs.xml 1 file changed, 57 insertions(+), 39 deletions(-) Approvals: Balazs Jeszenszky: Looks good to me, but someone else must approve Alex Rodoni: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10561 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I22926eb6050f9501624d2041f594fe4ef15be73b Gerrit-Change-Number: 10561 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-6917: Implement COMMENT ON TABLE/VIEW
Fredy Wijaya has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/10478 ) Change subject: IMPALA-6917: Implement COMMENT ON TABLE/VIEW .. IMPALA-6917: Implement COMMENT ON TABLE/VIEW This patch implements updating comment on a table or view. Syntax: COMMENT ON TABLE t IS 'comment' COMMENT ON VIEW v IS 'comment' Testing: - Added new front-end tests - Ran all front-end tests - Added new end-to-end tests - Ran end-to-end DDL tests Change-Id: I497c17342f79ff7c99931fd8a0ddec0c79303dbf --- M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/CommentOnDbStmt.java M fe/src/main/java/org/apache/impala/analysis/CommentOnStmt.java A fe/src/main/java/org/apache/impala/analysis/CommentOnTableOrViewStmt.java A fe/src/main/java/org/apache/impala/analysis/CommentOnTableStmt.java A fe/src/main/java/org/apache/impala/analysis/CommentOnViewStmt.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M tests/metadata/test_ddl.py M tests/metadata/test_ddl_base.py 13 files changed, 339 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/78/10478/6 -- To view, visit http://gerrit.cloudera.org:8080/10478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I497c17342f79ff7c99931fd8a0ddec0c79303dbf Gerrit-Change-Number: 10478 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6987: [DOCS] Refactor the INVALIDATE METADATA and REFRESH docs
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/10339 ) Change subject: IMPALA-6987: [DOCS] Refactor the INVALIDATE METADATA and REFRESH docs .. Patch Set 5: Hi Vuk, Could you review the latest patch that addressed your comments? Thank you! -- To view, visit http://gerrit.cloudera.org:8080/10339 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2124e14900d0f82569c061cc46006447bb054b36 Gerrit-Change-Number: 10339 Gerrit-PatchSet: 5 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 05 Jun 2018 20:39:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7110. Fix errors from error-prone
Hello Bharath Vissapragada, Philip Zeyliger, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10583 to look at the new patch set (#2). Change subject: IMPALA-7110. Fix errors from error-prone .. IMPALA-7110. Fix errors from error-prone * Fixes several cases of Preconditions.checkNotNull() which were ineffective. Instead, we should use Preconditions.checkState. * Fixed a case of synchronizing on a non-final class member, which is a bad practice. * Fixed a case of trying to stringify an array instead of using Joiner to join it to a human-readable representation. * Fixed a couple cases where the format string in String.format didn't match the number of arguments passed. * Fixed a test where PrimitiveType objects were being used to look up elements in a Map. This would always return null, so the test was ineffective. I elected not to enable error-prone in the default maven profile since it increases compilation time noticeably. It can be enabled by running 'mvn -Perrorprone'. Even with this patch there are a bunch of warnings -- this only addresses the issues that errorprone considers "errors". Change-Id: Ie9bfadecb5b92bba4fc7921a6f87f249ed90e771 --- M fe/pom.xml M fe/src/main/java/org/apache/impala/analysis/CreateUdfStmt.java M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java M fe/src/main/java/org/apache/impala/analysis/Path.java M fe/src/main/java/org/apache/impala/catalog/DiskIdMapper.java M fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java M fe/src/main/java/org/apache/impala/catalog/StructType.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java 11 files changed, 51 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/10583/2 -- To view, visit http://gerrit.cloudera.org:8080/10583 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie9bfadecb5b92bba4fc7921a6f87f249ed90e771 Gerrit-Change-Number: 10583 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7100: [DOCS] Consistent memory alloc across executor nodes
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10561 ) Change subject: IMPALA-7100: [DOCS] Consistent memory alloc across executor nodes .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-docs-submit/308/ -- To view, visit http://gerrit.cloudera.org:8080/10561 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I22926eb6050f9501624d2041f594fe4ef15be73b Gerrit-Change-Number: 10561 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 05 Jun 2018 20:36:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7100: [DOCS] Consistent memory alloc across executor nodes
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/10561 ) Change subject: IMPALA-7100: [DOCS] Consistent memory alloc across executor nodes .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10561 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I22926eb6050f9501624d2041f594fe4ef15be73b Gerrit-Change-Number: 10561 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 05 Jun 2018 20:36:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7110. Fix some warnings from error-prone
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10583 ) Change subject: IMPALA-7110. Fix some warnings from error-prone .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/10583/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10583/1//COMMIT_MSG@21 PS1, Line 21: I elected not to enable error-prone in the maven settings since it > I'll give this a try, though usually when I attempt to do this kind of thin turned out to be not that bad to add a profile so I just added to this patch http://gerrit.cloudera.org:8080/#/c/10583/1/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java File fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java: http://gerrit.cloudera.org:8080/#/c/10583/1/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java@85 PS1, Line 85: argTypes > interesting, wonder why it didn't catch this one. Will do. argTypes is a List, so the built-in toString actually works. My fix down below was on the wrong spot in the line - it was actually complaining about the exception stack trace and I mis-fixed the error. -- To view, visit http://gerrit.cloudera.org:8080/10583 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie9bfadecb5b92bba4fc7921a6f87f249ed90e771 Gerrit-Change-Number: 10583 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 05 Jun 2018 20:36:03 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7100: [DOCS] Consistent memory alloc across executor nodes
Balazs Jeszenszky has posted comments on this change. ( http://gerrit.cloudera.org:8080/10561 ) Change subject: IMPALA-7100: [DOCS] Consistent memory alloc across executor nodes .. Patch Set 2: Code-Review+1 Feel free to carry the +1 if you've addressed the comments. -- To view, visit http://gerrit.cloudera.org:8080/10561 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I22926eb6050f9501624d2041f594fe4ef15be73b Gerrit-Change-Number: 10561 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 05 Jun 2018 20:33:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5380: [DOCS] Added additional filesystems supported in URI
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/10551 ) Change subject: IMPALA-5380: [DOCS] Added additional filesystems supported in URI .. Patch Set 2: Hi Lars, Could you review the patch and approve the changes? Thanks! -- To view, visit http://gerrit.cloudera.org:8080/10551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I11d57efd564fb5779539a88d7b209cee93d313c6 Gerrit-Change-Number: 10551 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Tue, 05 Jun 2018 20:33:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7100: [DOCS] Consistent memory alloc across executor nodes
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/10561 ) Change subject: IMPALA-7100: [DOCS] Consistent memory alloc across executor nodes .. Patch Set 2: HI Balazs, Could you review my latest patch and approve? Thank you! -- To view, visit http://gerrit.cloudera.org:8080/10561 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I22926eb6050f9501624d2041f594fe4ef15be73b Gerrit-Change-Number: 10561 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 05 Jun 2018 20:32:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7050: [DOCS] Document the max serialized incremental stat size setting
Balazs Jeszenszky has posted comments on this change. ( http://gerrit.cloudera.org:8080/10457 ) Change subject: IMPALA-7050: [DOCS] Document the max serialized incremental stat size setting .. Patch Set 3: (7 comments) initial pass http://gerrit.cloudera.org:8080/#/c/10457/3/docs/topics/impala_perf_stats.xml File docs/topics/impala_perf_stats.xml: http://gerrit.cloudera.org:8080/#/c/10457/3/docs/topics/impala_perf_stats.xml@709 PS3, Line 709: When you need to run COMPUTE INCREMENTAL STATS on : very large tables, you can use the configuration setting : inc_stats_size_limit_bytes to reduce the load on : the catalog server. turn into passive? http://gerrit.cloudera.org:8080/#/c/10457/3/docs/topics/impala_perf_stats.xml@714 PS3, Line 714: This limit is set as a safety check, to prevent the JVM from hitting : a maximum array limit of 1 GB or runs out of memory. the limit is 2GB with suggested JVM versions. Readers would need to understand that this is only a part of the entire table's metadata (all of which together must be below 2GB when serialized as Thrift). So while we're trying to avoid a 2GB limit, increasing the 200MB limit can easily lead to crashes (other parts of the table's metadata can add up quickly). http://gerrit.cloudera.org:8080/#/c/10457/3/docs/topics/impala_perf_stats.xml@724 PS3, Line 724: To change the inc_stats_size_limit_bytes value This is a startup option for the impala daemons and the catalogd, not a client (impala-shell) side config. http://gerrit.cloudera.org:8080/#/c/10457/3/docs/topics/impala_perf_stats.xml@733 PS3, Line 733: 1 GB I'd put 500MB http://gerrit.cloudera.org:8080/#/c/10457/3/docs/topics/impala_perf_stats.xml@733 PS3, Line 733: spike in heap usage as well as crashes. http://gerrit.cloudera.org:8080/#/c/10457/3/docs/topics/impala_perf_stats.xml@736 PS3, Line 736: : Setting inc_stats_size_limit_bytes to a big value, : such as 1 GB or more, can result in a spike in heap usage. : duplicate of above? http://gerrit.cloudera.org:8080/#/c/10457/3/docs/topics/impala_shell_options.xml File docs/topics/impala_shell_options.xml: http://gerrit.cloudera.org:8080/#/c/10457/3/docs/topics/impala_shell_options.xml@570 PS3, Line 570: --inc_stats_size_limit_bytes this is an impalad-side option -- To view, visit http://gerrit.cloudera.org:8080/10457 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifa80325f0008d42a9cc8178e7c144fc2b49d7d4e Gerrit-Change-Number: 10457 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 05 Jun 2018 20:31:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10601 ) Change subject: IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang .. Patch Set 6: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2600/ -- To view, visit http://gerrit.cloudera.org:8080/10601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c147550f86d81b818ecbdd34cf2919ced7ff8c5 Gerrit-Change-Number: 10601 Gerrit-PatchSet: 6 Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jun 2018 20:26:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10601 ) Change subject: IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang .. Patch Set 5: Tim, please take a look at the changes to the test in patchset 5 vs patchset 3. -- To view, visit http://gerrit.cloudera.org:8080/10601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c147550f86d81b818ecbdd34cf2919ced7ff8c5 Gerrit-Change-Number: 10601 Gerrit-PatchSet: 5 Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jun 2018 20:26:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang
Hello Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10601 to look at the new patch set (#5). Change subject: IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang .. IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang If we hit EOS, we'll wait for all the backends to report status (to try to get a complete profile). But if the query is closed after this point, then we can get stuck waiting since once the query is closed, ImpalaServer won't know about this coordinator and so it will stop forwarding on the ReportStatus RPCs. The real fix for this is IMPALA-6984, but in the mean time, add another special case for this JIRA (see the other TODO IMPALA-6984 in coordinator.cc). The cancellation test only finds this race once in a while (several hours) indirectly in a COMPUTE STATS query because the ChildQueryExecutor will do a CloseOperation() while the execution thread is inside Fetch(). To make this more reproducible, modify the cancellation test to allow the close and fetch rpcs to execute concurrently (don't join the test's fetch thread until after close). This makes the race reproducible in a few iterations and a few minutes. Testing: - Loop test_cancellation.py Change-Id: I7c147550f86d81b818ecbdd34cf2919ced7ff8c5 --- M be/src/runtime/coordinator.cc M tests/query_test/test_cancellation.py 2 files changed, 47 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/10601/5 -- To view, visit http://gerrit.cloudera.org:8080/10601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7c147550f86d81b818ecbdd34cf2919ced7ff8c5 Gerrit-Change-Number: 10601 Gerrit-PatchSet: 5 Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10601 ) Change subject: IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang .. Patch Set 4: Oops, hold on one second. -- To view, visit http://gerrit.cloudera.org:8080/10601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c147550f86d81b818ecbdd34cf2919ced7ff8c5 Gerrit-Change-Number: 10601 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jun 2018 20:24:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10601 ) Change subject: IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang .. Patch Set 3: (3 comments) Tim, could you take a look at the new version of the test? http://gerrit.cloudera.org:8080/#/c/10601/3/tests/query_test/test_cancellation.py File tests/query_test/test_cancellation.py: http://gerrit.cloudera.org:8080/#/c/10601/3/tests/query_test/test_cancellation.py@a148 PS3, Line 148: > I realized last night that it's best if we sometimes keep this join here, s Done http://gerrit.cloudera.org:8080/#/c/10601/3/tests/query_test/test_cancellation.py@152 PS3, Line 152: # If the query is cancelled while it's in the fetch rpc, it gets unregistered and > This comment seems out-of-date Done http://gerrit.cloudera.org:8080/#/c/10601/3/tests/query_test/test_cancellation.py@230 PS3, Line 230: TestCancellatio > that should be TestCancellationFullSort (otherwise the super method is not Done -- To view, visit http://gerrit.cloudera.org:8080/10601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c147550f86d81b818ecbdd34cf2919ced7ff8c5 Gerrit-Change-Number: 10601 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jun 2018 20:23:44 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang
Hello Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10601 to look at the new patch set (#4). Change subject: IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang .. IMPALA-7101: Fix race between Fetch and Close RPCs that can lead to hang If we hit EOS, we'll wait for all the backends to report status (to try to get a complete profile). But if the query is closed after this point, then we can get stuck waiting since once the query is closed, ImpalaServer won't know about this coordinator and so it will stop forwarding on the ReportStatus RPCs. The real fix for this is IMPALA-6984, but in the mean time, add another special case for this JIRA (see the other TODO IMPALA-6984 in coordinator.cc). The cancellation test only finds this race once in a while (several hours) indirectly in a COMPUTE STATS query because the ChildQueryExecutor will do a CloseOperation() while the execution thread is inside Fetch(). To make this more reproducible, modify the cancellation test to allow the close and fetch rpcs to execute concurrently (don't join the test's fetch thread until after close). This makes the race reproducible in a few iterations and a few minutes. Testing: - Loop test_cancellation.py Change-Id: I7c147550f86d81b818ecbdd34cf2919ced7ff8c5 --- M be/src/runtime/coordinator.cc M tests/query_test/test_cancellation.py 2 files changed, 47 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/10601/4 -- To view, visit http://gerrit.cloudera.org:8080/10601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7c147550f86d81b818ecbdd34cf2919ced7ff8c5 Gerrit-Change-Number: 10601 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6812: Fix flaky Kudu scan tests
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/10503 ) Change subject: IMPALA-6812: Fix flaky Kudu scan tests .. Patch Set 1: READ_AT_SNAPSHOT was reverted because it caused flakyness in stress tests iirc. Note also that both READ_AT_SNAPSHOT and READ_YOUR_WRITES need timestamp propagation between clients/queries to work properly. READ_AT_SNAPSHOT, while not consistent without timestamp propagation, effectively provided RYW by forcing the read to happen at now(), forcibly after the last write and without coordination. But READ_AT_SNAPSHOT requires an effective leader to work, which were sometimes missing in stress tests. I realize that impala added a kudu client cache to the backend. If that makes it so that the same client is used across different queries, then RYW should both solve consistency issues and be more lenient regarding leaderless tablets. If the same client is not always used, then RYW might help, but likely won't completely solve the problem. -- To view, visit http://gerrit.cloudera.org:8080/10503 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I70df84f2cbc663107f2ad029565d3c15bdfbd47c Gerrit-Change-Number: 10503 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 05 Jun 2018 18:54:47 + Gerrit-HasComments: No
[Impala-ASF-CR] Add missing namespace qualifiers
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10608 ) Change subject: Add missing namespace qualifiers .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2599/ -- To view, visit http://gerrit.cloudera.org:8080/10608 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ef8f5c8f05100418dec90c2cd2e2b3dcd827c27 Gerrit-Change-Number: 10608 Gerrit-PatchSet: 1 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jun 2018 18:48:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7120: remove sonatype repository
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10606 to look at the new patch set (#2). Change subject: IMPALA-7120: remove sonatype repository .. IMPALA-7120: remove sonatype repository >From what I can tell, we no longer need this because we get java-cup from a different repository. I started a build and we built the frontend ok, so unless I'm missing something, that seems to confirm that it works. Change-Id: I8b7acee041019eecc8f664f2093dd3df72341a7e --- M impala-parent/pom.xml 1 file changed, 0 insertions(+), 26 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/10606/2 -- To view, visit http://gerrit.cloudera.org:8080/10606 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8b7acee041019eecc8f664f2093dd3df72341a7e Gerrit-Change-Number: 10606 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] Add missing namespace qualifiers
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10608 ) Change subject: Add missing namespace qualifiers .. Patch Set 1: Code-Review+2 Thanks! Seems ok to go in now. -- To view, visit http://gerrit.cloudera.org:8080/10608 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ef8f5c8f05100418dec90c2cd2e2b3dcd827c27 Gerrit-Change-Number: 10608 Gerrit-PatchSet: 1 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 05 Jun 2018 18:43:50 + Gerrit-HasComments: No
[Impala-ASF-CR] Add missing namespace qualifiers
Lars Volker has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10608 Change subject: Add missing namespace qualifiers .. Add missing namespace qualifiers As part of an upcoming rebase of the KRPC code, this change adds missing namespace qualifiers in some places. Change-Id: I3ef8f5c8f05100418dec90c2cd2e2b3dcd827c27 --- M be/src/exec/kudu-util.h M be/src/runtime/descriptors.h 2 files changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/10608/1 -- To view, visit http://gerrit.cloudera.org:8080/10608 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I3ef8f5c8f05100418dec90c2cd2e2b3dcd827c27 Gerrit-Change-Number: 10608 Gerrit-PatchSet: 1 Gerrit-Owner: Lars Volker
[Impala-ASF-CR] Fix Indents from IMPALA-4970
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10560 ) Change subject: Fix Indents from IMPALA-4970 .. Patch Set 2: Thanks! Sorry to be pedantic -- To view, visit http://gerrit.cloudera.org:8080/10560 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4ea53e0780cca9f791808c2a73c7f72ba27c2159 Gerrit-Change-Number: 10560 Gerrit-PatchSet: 2 Gerrit-Owner: Rahul Shivu Mahadev Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Rahul Shivu Mahadev Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 05 Jun 2018 18:38:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5737: Tighten minicluster memory limit
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10277 ) Change subject: IMPALA-5737: Tighten minicluster memory limit .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2598/ -- To view, visit http://gerrit.cloudera.org:8080/10277 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If8240551e726c6da546a926a1ce3444f41ef87fe Gerrit-Change-Number: 10277 Gerrit-PatchSet: 7 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Comment-Date: Tue, 05 Jun 2018 18:37:14 + Gerrit-HasComments: No
