[Impala-ASF-CR] IMPALA-9026: Use resolved IP address for statestore subscriber
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/14388 ) Change subject: IMPALA-9026: Use resolved IP address for statestore subscriber .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/14388/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14388/3//COMMIT_MSG@17 PS3, Line 17: muplitle > typo Done -- To view, visit http://gerrit.cloudera.org:8080/14388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieb8302dec0e52beb9f0b88306a51c38ff42a63a2 Gerrit-Change-Number: 14388 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 23 Oct 2019 05:33:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9026: Use resolved IP address for statestore subscriber
Hello Sahil Takiar, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14388 to look at the new patch set (#4). Change subject: IMPALA-9026: Use resolved IP address for statestore subscriber .. IMPALA-9026: Use resolved IP address for statestore subscriber This change adds a flag (--statestore_subscriber_use_resolved_address) which, if set to true, allows statestore subscribers to use its resolved IP address instead of its hostname as the heartbeat address which statestore sends heartbeats / updates to. This flag is useful in certain situation in which the subscriber's DNS entry may not be present for a valid reason (e.g. a Kubernetes pod whose readiness probe returns false). An example is that there are multiple Impala coordinators but only one of them will be active at a time (for admission control reason) and the rest will serve as backup. In which case, we still want the backup coordinators to receive updates from statestore but not serve any queries. Change-Id: Ieb8302dec0e52beb9f0b88306a51c38ff42a63a2 --- M be/src/catalog/catalog-server.cc M be/src/runtime/exec-env.cc M be/src/statestore/statestore-subscriber.cc M be/src/statestore/statestore-subscriber.h M tests/custom_cluster/test_custom_statestore.py 5 files changed, 35 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/14388/4 -- To view, visit http://gerrit.cloudera.org:8080/14388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ieb8302dec0e52beb9f0b88306a51c38ff42a63a2 Gerrit-Change-Number: 14388 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-9026: Use resolved IP address for statestore subscriber
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14388 ) Change subject: IMPALA-9026: Use resolved IP address for statestore subscriber .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4857/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieb8302dec0e52beb9f0b88306a51c38ff42a63a2 Gerrit-Change-Number: 14388 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 23 Oct 2019 03:01:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9026: Use resolved IP address for statestore subscriber
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/14388 ) Change subject: IMPALA-9026: Use resolved IP address for statestore subscriber .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/14388/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14388/3//COMMIT_MSG@17 PS3, Line 17: muplitle typo -- To view, visit http://gerrit.cloudera.org:8080/14388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieb8302dec0e52beb9f0b88306a51c38ff42a63a2 Gerrit-Change-Number: 14388 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 23 Oct 2019 02:17:35 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9026: Use resolved IP address for statestore subscriber
Michael Ho has posted comments on this change. (
http://gerrit.cloudera.org:8080/14388 )
Change subject: IMPALA-9026: Use resolved IP address for statestore subscriber
..
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14388/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14388/2//COMMIT_MSG@19
PS2, Line 19:
> nit: line too long
Done
http://gerrit.cloudera.org:8080/#/c/14388/2/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:
http://gerrit.cloudera.org:8080/#/c/14388/2/be/src/runtime/exec-env.cc@222
PS2, Line 222: tmp_file_mgr_(new TmpFileMgr),
: frontend_(new Frontend()),
: async_rpc_pool_(new CallableThreadPool("rpc-pool",
"async-rpc-sender", 8, 1)),
: query_exec_mgr_(new QueryExecMgr()),
: rpc_metrics_(metrics_->GetOrCreateChildG
> whats the reason for moving these from Init() to the constructor?
Failing in Init() below or here are both fatal (i.e. Impala will not start) so
I may as well remove the complication of splitting the initialization of
krpc_address_ into two functions.
In a previous version of the patch I needed the IP address in this function to
initialize the statestore subscriber but has since modified the code but I feel
the simplification above is still warranted.
http://gerrit.cloudera.org:8080/#/c/14388/2/be/src/statestore/statestore-subscriber.cc
File be/src/statestore/statestore-subscriber.cc:
http://gerrit.cloudera.org:8080/#/c/14388/2/be/src/statestore/statestore-subscriber.cc@262
PS2, Line 262: hostname
> document that the hostname can now either be the actual hostname, or the ip
Comments updated in the header filer. Seems simpler to just resolve the
hostname here instead of having the server choose between multiple addresses.
--
To view, visit http://gerrit.cloudera.org:8080/14388
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb8302dec0e52beb9f0b88306a51c38ff42a63a2
Gerrit-Change-Number: 14388
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Sahil Takiar
Gerrit-Comment-Date: Wed, 23 Oct 2019 02:16:40 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9026: Use resolved IP address for statestore subscriber
Hello Sahil Takiar, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14388 to look at the new patch set (#3). Change subject: IMPALA-9026: Use resolved IP address for statestore subscriber .. IMPALA-9026: Use resolved IP address for statestore subscriber This change adds a flag (--statestore_subscriber_use_resolved_address) which, if set to true, allows statestore subscribers to use its resolved IP address instead of its hostname as the heartbeat address which statestore sends heartbeats / updates to. This flag is useful in certain situation in which the subscriber DNS entry may not be present for a valid reason (e.g. a Kubernetes pod whose readiness probe returns false). An example is that there are muplitle Impala coordinator but only one of them will be active at a time (for admission control reasons) and the rest will serve as backup. In which case, we still want the backup coordinators to receive updates from statestore but not serve any queries. Change-Id: Ieb8302dec0e52beb9f0b88306a51c38ff42a63a2 --- M be/src/catalog/catalog-server.cc M be/src/runtime/exec-env.cc M be/src/statestore/statestore-subscriber.cc M be/src/statestore/statestore-subscriber.h M tests/custom_cluster/test_custom_statestore.py 5 files changed, 35 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/14388/3 -- To view, visit http://gerrit.cloudera.org:8080/14388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ieb8302dec0e52beb9f0b88306a51c38ff42a63a2 Gerrit-Change-Number: 14388 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5 parse name() instead
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14433 ) Change subject: IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5_parse_name() instead .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4856/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14433 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24 Gerrit-Change-Number: 14433 Gerrit-PatchSet: 6 Gerrit-Owner: Xiaomeng Zhang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Xiaomeng Zhang Gerrit-Comment-Date: Wed, 23 Oct 2019 01:55:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5 parse name() instead
Xiaomeng Zhang has posted comments on this change. (
http://gerrit.cloudera.org:8080/14433 )
Change subject: IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use
krb5_parse_name() instead
..
Patch Set 6:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@10
PS5, Line 10: using custom code.
> instead of using custom code.
Done
http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@12
PS5, Line 12: When kerberos is initialized in Impala's copy of Kudu code, it
stores a
> The reader of commit messages is developers like you and me.
Done
http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@18
PS5, Line 18: Test done:
> to verify parsing a principal nae containing a special character.
Done
http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@19
PS5, Line 19: Add two authentication-test, one to verify parsing a principal
containing
> Is it possible to have an automated test for this?
Yes, added a new test BadPrincipalFormat
http://gerrit.cloudera.org:8080/#/c/14433/5//COMMIT_MSG@21
PS5, Line 21: format principal, new error code is 2 instead of original 112
> Do you want to mention running end-to-end tests?
What kind of end-to-end test? The one included in impala-private-parameterized?
http://gerrit.cloudera.org:8080/#/c/14433/5/be/src/kudu/security/init.h
File be/src/kudu/security/init.h:
http://gerrit.cloudera.org:8080/#/c/14433/5/be/src/kudu/security/init.h@39
PS5, Line 39: // Parses the given Kerberos principal into service name,
hostname, and realm.
> Maybe: "Parse a kerberos principal name and extract the ervice_name, hostna
Done
http://gerrit.cloudera.org:8080/#/c/14433/5/be/src/util/auth-util.cc
File be/src/util/auth-util.cc:
http://gerrit.cloudera.org:8080/#/c/14433/5/be/src/util/auth-util.cc@92
PS5, Line 92: hostname, realm), strings::Substitute("bad principal format
$0", principal));
> It is more idiomatic and marginally more efficient to use:
Done
--
To view, visit http://gerrit.cloudera.org:8080/14433
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24
Gerrit-Change-Number: 14433
Gerrit-PatchSet: 6
Gerrit-Owner: Xiaomeng Zhang
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Xiaomeng Zhang
Gerrit-Comment-Date: Wed, 23 Oct 2019 01:13:17 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5 parse name() instead
Xiaomeng Zhang has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/14433 ) Change subject: IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5_parse_name() instead .. IMPALA-7504/KUDU-2979 ParseKerberosPrincipal() should use krb5_parse_name() instead We want to use krb5_parse_name() to parse the principal instead of using custom code. When kerberos is initialized in Impala's copy of Kudu code, it stores a global context which is used when accessing the Krb5 library. To use this global context the code that parses the principal name is moved into the Impala Kudu code. This new code is then called from the existing ParseKerberosPrincipal method. Test done: Add two authentication-test, one to verify parsing a principal containing a special character. The other to verify exception when parsing bad format principal, new error code is 2 instead of original 112 which is BAD_PRINCIPAL_FORMAT error code. Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24 --- M be/src/kudu/security/init.cc M be/src/kudu/security/init.h M be/src/rpc/authentication-test.cc M be/src/util/auth-util.cc 4 files changed, 41 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/14433/6 -- To view, visit http://gerrit.cloudera.org:8080/14433 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e64ebdc10f102dbdc5b87f6fe3f2a0310b1be24 Gerrit-Change-Number: 14433 Gerrit-PatchSet: 6 Gerrit-Owner: Xiaomeng Zhang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Xiaomeng Zhang
[Impala-ASF-CR] IMPALA-8065 OSInfo produces somewhat misleading output when running in container
Andrew Sherman has posted comments on this change. (
http://gerrit.cloudera.org:8080/14531 )
Change subject: IMPALA-8065 OSInfo produces somewhat misleading output when
running in container
..
Patch Set 1:
(2 comments)
A couple of quick notes
http://gerrit.cloudera.org:8080/#/c/14531/1/be/src/util/os-info.cc
File be/src/util/os-info.cc:
http://gerrit.cloudera.org:8080/#/c/14531/1/be/src/util/os-info.cc@56
PS1, Line 56: ifstream os_version("/etc/os-release", ios::in);
I think this file is not present on Centos6.
I *think* we still support that, though I am not 100% sure.
http://gerrit.cloudera.org:8080/#/c/14531/1/be/src/util/os-info.cc@99
PS1, Line 99: << "Kernel version: " << kernel_version_ << endl
If I were reading this I would be confused: what is OS version, what is Kernel
version? Are they the same?
If I am not in Docker what do I see?
My suggestion: only if we are running in Docker print
"Container OS: "
--
To view, visit http://gerrit.cloudera.org:8080/14531
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I848c9e53ee4e0bf8ae0874bb6da28e8efa7f7c8a
Gerrit-Change-Number: 14531
Gerrit-PatchSet: 1
Gerrit-Owner: Xiaomeng Zhang
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 23 Oct 2019 01:00:28 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9027: planner fixes for mt dop
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14522 to look at the new patch set (#4). Change subject: IMPALA-9027: planner fixes for mt_dop .. IMPALA-9027: planner fixes for mt_dop * Update the distributed planner to reflect that broadcast join tables are replicated in all fragments. * Did a pass over the planner code looking at call sites of getNumNodes() to confirm that they shouldn't be replaced by getNumInstances() Testing: * Updated affected planner test where PARALLELPLANS had a different join strategy. * Added a targeted test to mem-limit-broadcast-join.test to show that mt_dop affects join mode. * Ran exhaustive tests. Change-Id: I23395c2dadf6be0e8be99706ca3ab5f4964cbcf9 --- M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/HashJoinNode.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M testdata/workloads/functional-planner/queries/PlannerTest/mem-limit-broadcast-join.test M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test M testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test 9 files changed, 769 insertions(+), 667 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/14522/4 -- To view, visit http://gerrit.cloudera.org:8080/14522 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I23395c2dadf6be0e8be99706ca3ab5f4964cbcf9 Gerrit-Change-Number: 14522 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7506: support global INVALIDATE METADATA in local catalog mode
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/14307 )
Change subject: IMPALA-7506: support global INVALIDATE METADATA in local
catalog mode
..
Patch Set 8:
(3 comments)
I generally like this solution -- seems simpler than trying to track the full
contents of the cache.
Is it possible to use this solution for catalog v1 as well, and remove the
CatalogObjectVersionSet thing? I think that thing is buggy/complex, maybe would
be nice to just have one implementation to worry about. (ok to do that in a
follow-up if it's not trivial, or if you think it's a risky change, maybe we
shouldn't do it at all)
http://gerrit.cloudera.org:8080/#/c/14307/8/be/src/util/impalad-metrics.h
File be/src/util/impalad-metrics.h:
http://gerrit.cloudera.org:8080/#/c/14307/8/be/src/util/impalad-metrics.h@119
PS8, Line 119: /// Minimal catalog object version in local catalog cache of
Coordinator.
this isn't actually true -- it's no longer the minimum, but rather a lower
bound, right? ie it's not a tight bound like it used to be in v1. We should
probably rename the metric, variables, thrift fields, etc, to reflect that.
http://gerrit.cloudera.org:8080/#/c/14307/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/14307/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1587
PS8, Line 1587: lastResetStartVersion_ = startVersion;
What if there are two concurrent calls to INVALIDATE METADATA? is it possible
that the lastResetStartVersion_ will end up going downward, because the setting
of startVersion is under a different lock acquisition from the version
writelock above?
http://gerrit.cloudera.org:8080/#/c/14307/8/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:
http://gerrit.cloudera.org:8080/#/c/14307/8/tests/authorization/test_grant_revoke.py@364
PS8, Line 364: self.client.execute("grant role {0} to group
foobar".format(role_name))
why were these changes necessary?
--
To view, visit http://gerrit.cloudera.org:8080/14307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61a7ab1ffa062620ffbc2dadc34bd7a8ca9e549
Gerrit-Change-Number: 14307
Gerrit-PatchSet: 8
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Tue, 22 Oct 2019 23:18:34 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8065 OSInfo produces somewhat misleading output when running in container
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14531 ) Change subject: IMPALA-8065 OSInfo produces somewhat misleading output when running in container .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4855/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14531 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I848c9e53ee4e0bf8ae0874bb6da28e8efa7f7c8a Gerrit-Change-Number: 14531 Gerrit-PatchSet: 1 Gerrit-Owner: Xiaomeng Zhang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 22 Oct 2019 23:14:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9047: Bump CDP BUILD NUMBER to 1507246
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14529 ) Change subject: IMPALA-9047: Bump CDP_BUILD_NUMBER to 1507246 .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/14529/1/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java File fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java: http://gerrit.cloudera.org:8080/#/c/14529/1/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@59 PS1, Line 59: import static org.junit.Assume.*; nit: we don't allow using * in import. Please specify the exact class name. -- To view, visit http://gerrit.cloudera.org:8080/14529 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifbc42a73fe98c437edc20a8f57aba51e3096a09d Gerrit-Change-Number: 14529 Gerrit-PatchSet: 1 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 22 Oct 2019 23:12:40 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7984: Port runtime filter from Thrift RPC to KRPC
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13882 ) Change subject: IMPALA-7984: Port runtime filter from Thrift RPC to KRPC .. Patch Set 25: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4854/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13882 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6b394796d250286510e157ae326882bfc01d387a Gerrit-Change-Number: 13882 Gerrit-PatchSet: 25 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 22 Oct 2019 22:32:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8065 OSInfo produces somewhat misleading output when running in container
Xiaomeng Zhang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14531 Change subject: IMPALA-8065 OSInfo produces somewhat misleading output when running in container .. IMPALA-8065 OSInfo produces somewhat misleading output when running in container Original we get /proc/version dispalyed as OS version, while it's actually kernel version. We should correct it as Kernel version, and display OS version from /etc/os-release. Tested locally, the displayed OS Info in localhost:25020 is: OS version: "Ubuntu 16.04.6 LTS" Kernel version: Linux version 4.15.0-65-generic (buildd@lcy01-amd64-017) (gcc version 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.10)) Clock: clocksource: 'tsc', clockid_t: CLOCK_MONOTONIC Change-Id: I848c9e53ee4e0bf8ae0874bb6da28e8efa7f7c8a --- M be/src/util/os-info.cc M be/src/util/os-info.h 2 files changed, 29 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/14531/1 -- To view, visit http://gerrit.cloudera.org:8080/14531 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I848c9e53ee4e0bf8ae0874bb6da28e8efa7f7c8a Gerrit-Change-Number: 14531 Gerrit-PatchSet: 1 Gerrit-Owner: Xiaomeng Zhang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7984: Port runtime filter from Thrift RPC to KRPC
Fang-Yu Rao has uploaded a new patch set (#25). ( http://gerrit.cloudera.org:8080/13882 ) Change subject: IMPALA-7984: Port runtime filter from Thrift RPC to KRPC .. IMPALA-7984: Port runtime filter from Thrift RPC to KRPC Previously the aggregation and propagation of a runtime filter in Impala is implemented using Thrift RPC, which suffers from a disadvantage that the number of connections in a cluster grows with both the number of queries and cluster size. This patch ports the functions that implement the aggregation and propagation of a runtime filter, i.e., UpdateFilter() and PublishFilter(), respctively, to KRPC, which requires only one connection per direction between every pair of hosts, thus reducing the number of connections in a cluster. In addition, this patch also incorporates KRPC sidecar when the runtime filter is a Bloom filter. KRPC sidecar eliminates the need for an extra copy of the Bloom filter contents when a Bloom filter is serialized to be transmitted and hence reduces the serialization overhead. Due to the incorporation of KRPC sidecar, a SpinLock is also added to prevent a BloomFilter from being deallocated before its associated KRPC call finishes. Two related BE tests bloom-filter-test.cc and bloom-filter-benchmark.cc are also modified accordingly because of the changes to the signatures of some functions in BloomFilter. Testing: This patch has passed the exhaustive tests. Change-Id: I6b394796d250286510e157ae326882bfc01d387a --- M be/src/benchmarks/bloom-filter-benchmark.cc M be/src/runtime/backend-client.h M be/src/runtime/client-cache.cc M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-backend-state.h M be/src/runtime/coordinator-filter-state.h M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/data-stream-test.cc M be/src/runtime/decimal-value.h M be/src/runtime/decimal-value.inline.h M be/src/runtime/exec-env.cc M be/src/runtime/fragment-instance-state.cc M be/src/runtime/fragment-instance-state.h M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-filter-bank.h M be/src/runtime/runtime-filter.h M be/src/runtime/timestamp-value.h M be/src/scheduling/request-pool-service.h M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/data-stream-service.cc M be/src/service/data-stream-service.h M be/src/service/frontend.h M be/src/service/impala-internal-service.cc M be/src/service/impala-internal-service.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/bloom-filter-test.cc M be/src/util/bloom-filter.cc M be/src/util/bloom-filter.h M be/src/util/min-max-filter-test.cc M be/src/util/min-max-filter.cc M be/src/util/min-max-filter.h M common/protobuf/common.proto M common/protobuf/data_stream_service.proto M common/thrift/ImpalaInternalService.thrift 39 files changed, 1,086 insertions(+), 747 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/13882/25 -- To view, visit http://gerrit.cloudera.org:8080/13882 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6b394796d250286510e157ae326882bfc01d387a Gerrit-Change-Number: 13882 Gerrit-PatchSet: 25 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-9065: don't block indefinitely for filters
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14499 ) Change subject: IMPALA-9065: don't block indefinitely for filters .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4853/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14499 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0a70e4451c2b48c97f854246e90b71f6e5d67710 Gerrit-Change-Number: 14499 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 22 Oct 2019 18:00:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9065: don't block indefinitely for filters
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14499 ) Change subject: IMPALA-9065: don't block indefinitely for filters .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4852/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14499 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0a70e4451c2b48c97f854246e90b71f6e5d67710 Gerrit-Change-Number: 14499 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 22 Oct 2019 17:59:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9047: Bump CDP BUILD NUMBER to 1507246
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14529 ) Change subject: IMPALA-9047: Bump CDP_BUILD_NUMBER to 1507246 .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4851/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14529 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifbc42a73fe98c437edc20a8f57aba51e3096a09d Gerrit-Change-Number: 14529 Gerrit-PatchSet: 1 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Tue, 22 Oct 2019 17:29:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9065: don't block indefinitely for filters
Tim Armstrong has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/14499 ) Change subject: IMPALA-9065: don't block indefinitely for filters .. IMPALA-9065: don't block indefinitely for filters This patch ensures that query cancellation will promptly wake up any threads blocked waiting for runtime filters to arrive. Before this patch, threads would wait for up to RUNTIME_FILTER_WAIT_TIME_MS after the query was cancelled. Testing: * Add a cancellation test with a high runtime filter wait time that reproduces the threads getting stuck. This test failed reliably without the code changes. * Also update metric verification to check that no fragments are left running when tests are finished. * Ran exhaustive tests. * Ran a 1 query TPC-H Kudu and TPC-DS Parquet stress test on a minicluster with 3 impalads. TODO: TPC-DS part. Change-Id: I0a70e4451c2b48c97f854246e90b71f6e5d67710 --- M be/src/exec/buffered-plan-root-sink.h M be/src/runtime/fragment-instance-state.cc M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-filter-bank.h M be/src/runtime/runtime-filter.cc M be/src/runtime/runtime-filter.h M be/src/runtime/runtime-filter.inline.h M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M tests/query_test/test_runtime_filters.py M tests/verifiers/metric_verifier.py 11 files changed, 141 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/14499/7 -- To view, visit http://gerrit.cloudera.org:8080/14499 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0a70e4451c2b48c97f854246e90b71f6e5d67710 Gerrit-Change-Number: 14499 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-9065: don't block indefinitely for filters
Hello Thomas Tauber-Marshall, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14499 to look at the new patch set (#8). Change subject: IMPALA-9065: don't block indefinitely for filters .. IMPALA-9065: don't block indefinitely for filters This patch ensures that query cancellation will promptly wake up any threads blocked waiting for runtime filters to arrive. Before this patch, threads would wait for up to RUNTIME_FILTER_WAIT_TIME_MS after the query was cancelled. Testing: * Add a cancellation test with a high runtime filter wait time that reproduces the threads getting stuck. This test failed reliably without the code changes. * Also update metric verification to check that no fragments are left running when tests are finished. * Ran exhaustive tests. * Ran a 1 query TPC-H Kudu and TPC-DS Parquet stress test on a minicluster with 3 impalads. TODO: TPC-DS part. Change-Id: I0a70e4451c2b48c97f854246e90b71f6e5d67710 --- M be/src/exec/buffered-plan-root-sink.h M be/src/runtime/fragment-instance-state.cc M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-filter-bank.h M be/src/runtime/runtime-filter.cc M be/src/runtime/runtime-filter.h M be/src/runtime/runtime-filter.inline.h M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M tests/query_test/test_runtime_filters.py M tests/verifiers/metric_verifier.py 11 files changed, 139 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/14499/8 -- To view, visit http://gerrit.cloudera.org:8080/14499 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0a70e4451c2b48c97f854246e90b71f6e5d67710 Gerrit-Change-Number: 14499 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-9047: Bump CDP BUILD NUMBER to 1507246
Fang-Yu Rao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14529 Change subject: IMPALA-9047: Bump CDP_BUILD_NUMBER to 1507246 .. IMPALA-9047: Bump CDP_BUILD_NUMBER to 1507246 This patch bumps CDP_BUILD_NUMBER to 1507246. Some test cases would fail due to this change. For example, some Ranger related E2E tests and FE tests would fail when we are using a newer version of Ranger (e.g., ranger-2.0.0.7.0.2.0-108 instead of ranger-1.2.0.7.1.0.0-33) due to the changes to the default Ranger policies described at https://issues.apache.org/jira/browse/RANGER-2536. To address this issue, this patch temporarily disables those affected Ranger tests. Specifically, the affected tests in the following test files are disabled for now. 1. test_authorized_proxy.py 2. test_ranger.py 3. AuthorizationStmtTest.java 4. RangerAuditLogTest.java Testing: - This patch passes the affected Ranger tests listed above on a local machine. Change-Id: Ifbc42a73fe98c437edc20a8f57aba51e3096a09d --- M bin/impala-config.sh M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java M tests/authorization/test_authorized_proxy.py M tests/authorization/test_ranger.py 5 files changed, 101 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/14529/1 -- To view, visit http://gerrit.cloudera.org:8080/14529 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ifbc42a73fe98c437edc20a8f57aba51e3096a09d Gerrit-Change-Number: 14529 Gerrit-PatchSet: 1 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Joe McDonnell
[Impala-ASF-CR] IMPALA-9047: Bump CDP BUILD NUMBER to 1523729
Fang-Yu Rao has abandoned this change. ( http://gerrit.cloudera.org:8080/14508 ) Change subject: IMPALA-9047: Bump CDP_BUILD_NUMBER to 1523729 .. Abandoned Will bump CDP_BUILD_NUMBER to 1507246 instead -- To view, visit http://gerrit.cloudera.org:8080/14508 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I9c4ddc846ca879eae6ca4bce9bac77d7a9a3f43e Gerrit-Change-Number: 14508 Gerrit-PatchSet: 1 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell
[Impala-ASF-CR] [WIP] IMPALA-9071: Handle translated external HDFS table in CATS
Quanlong Huang has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/14527 ) Change subject: [WIP] IMPALA-9071: Handle translated external HDFS table in CATS .. [WIP] IMPALA-9071: Handle translated external HDFS table in CATS After upgrading Hive-3 to a version containing HIVE-22158, it's not allowed for managed tables to be non transactional. Creating non ACID tables will result in creating an external table with table property 'external.table.purge' set to true. In Hive-3, the default location of external HDFS tables will locate in 'metastore.warehouse.external.dir' if it's set. This property is added by HIVE-19837 in Hive 2.7, but hasn't been added to Hive in cdh6 yet. In CTAS statement, we create a temporary HMS Table for the analysis on the Insert part. The table path is created assuming it's a managed table, and the Insert part will use this path for insertion. However, in Hive-3, the created table is translated to an external table. It's not the same as we passed to the HMS API. The created table is located in 'metastore.warehouse.external.dir', while the table path we assumed is in 'metastore.warehouse.dir'. This introduces bugs when these two properties are different. CTAS statement will create table in one place and insert data in another place. This patch adds a new method in MetastoreShim to wrap the difference for getting the default table path for non transactional tables between Hive-2 and Hive-3. This patch also bumps the CDP version to contain HIVE-22158. Test Plan: - Adding a custom cluster test to start Hive with external.dir being set. Change-Id: I460a57dc877ef68ad7dd0864a33b1599b1e9a8d9 --- M bin/impala-config.sh M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M testdata/pom.xml 5 files changed, 55 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/14527/2 -- To view, visit http://gerrit.cloudera.org:8080/14527 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I460a57dc877ef68ad7dd0864a33b1599b1e9a8d9 Gerrit-Change-Number: 14527 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang
[Impala-ASF-CR] IMPALA-6501: Optimize count(star) for Kudu scans
Daniel Becker has posted comments on this change. (
http://gerrit.cloudera.org:8080/14347 )
Change subject: IMPALA-6501: Optimize count(star) for Kudu scans
..
Patch Set 13:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14347/10/tests/query_test/test_aggregation.py
File tests/query_test/test_aggregation.py:
http://gerrit.cloudera.org:8080/#/c/14347/10/tests/query_test/test_aggregation.py@273
PS10, Line 273:vector.get_value('table_format').compression_codec !=
'none'):
> That would result in:
I didn't think of that, thanks for checking.
--
To view, visit http://gerrit.cloudera.org:8080/14347
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic99e0f954d0ca65779bd531ca79ace1fcb066fb9
Gerrit-Change-Number: 14347
Gerrit-PatchSet: 13
Gerrit-Owner: Norbert Luksa
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Norbert Luksa
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Tue, 22 Oct 2019 15:12:00 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6663 Expose current DDL metrics on WebUI
Zoltan Borok-Nagy has posted comments on this change. (
http://gerrit.cloudera.org:8080/13806 )
Change subject: IMPALA-6663 Expose current DDL metrics on WebUI
..
Patch Set 7:
(6 comments)
Found a couple of nits, other than that lgtm. It will be even more useful when
you add the table names to the metrics.
http://gerrit.cloudera.org:8080/#/c/13806/7/be/src/catalog/catalog-server.cc
File be/src/catalog/catalog-server.cc:
http://gerrit.cloudera.org:8080/#/c/13806/7/be/src/catalog/catalog-server.cc@545
PS7, Line 545: int i = 0; i < catalog_usage_result.ddl_type_counter.size(); ++i
nit: range-based for loop can be used instead:
for (const auto& ddl_type_counter : catalog_usage_result.ddl_type_counter) {
..
}
http://gerrit.cloudera.org:8080/#/c/13806/7/common/thrift/JniCatalog.thrift
File common/thrift/JniCatalog.thrift:
http://gerrit.cloudera.org:8080/#/c/13806/7/common/thrift/JniCatalog.thrift@718
PS7, Line 718: dll
typo: ddl
http://gerrit.cloudera.org:8080/#/c/13806/7/common/thrift/JniCatalog.thrift@720
PS7, Line 720:
nit: unnecessary blank line
http://gerrit.cloudera.org:8080/#/c/13806/7/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogOpRequestCounter.java
File
fe/src/main/java/org/apache/impala/catalog/monitor/CatalogOpRequestCounter.java:
http://gerrit.cloudera.org:8080/#/c/13806/7/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogOpRequestCounter.java@62
PS7, Line 62: if (isSyncDdl) {
: syncDdlOperationCounter_.incrementAndGet();
: }
nit: this and the other 'if (isSyncDdl)' statements in this file would fit in a
single line.
http://gerrit.cloudera.org:8080/#/c/13806/7/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java
File
fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java:
http://gerrit.cloudera.org:8080/#/c/13806/7/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogTableMetrics.java@26
PS7, Line 26: import com.google.common.base.Function;
it's also imported at L20.
http://gerrit.cloudera.org:8080/#/c/13806/7/fe/src/main/java/org/apache/impala/service/JniCatalog.java
File fe/src/main/java/org/apache/impala/service/JniCatalog.java:
http://gerrit.cloudera.org:8080/#/c/13806/7/fe/src/main/java/org/apache/impala/service/JniCatalog.java@95
PS7, Line 95: catalogUsageMonitor
nit: maybe you could create a reference to the CatalogOpRequestCounter object
since you only use that later.
This way all the increments and decrements will fit in a single line.
--
To view, visit http://gerrit.cloudera.org:8080/13806
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0ed76f134bad6d3b3d4dce132365a53a01e9512a
Gerrit-Change-Number: 13806
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tamas Mate
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Tue, 22 Oct 2019 12:35:02 +
Gerrit-HasComments: Yes
