impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Tauber-Marshall (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5167: Reduce the number of Kudu clients created (BE)
Date Tue, 16 May 2017 16:26:06 GMT
Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-5167: Reduce the number of Kudu clients created (BE)
......................................................................


Patch Set 6:

(3 comments)

> So I just spoke with the Kudu folks and it sounds like there will
 > be an issue with the Java client that will prevent it from being
 > used for more than 7 days w/ Kerberos (some token issue), and I
 > don't think that'll get fixed ASAP. The issue does exist for the
 > C++ client as well but that's being fixed now. Can you separate out
 > the Java side so we can submit them independently?
Done.

http://gerrit.cloudera.org:8080/#/c/6792/5/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:

Line 403:     KuduClientPtr* kudu_client_ptr = new KuduClientPtr;
> no std:: in .cc
Done


Line 404:     RETURN_IF_ERROR(CreateKuduClient(master_addresses, &kudu_client_ptr->kudu_client));
> I suggested it, but we can take it out if you think it's superfluous.
If I understand correctly, this list comes from a table property, which by default is populated
from a start up flag. I assume the common case we expect is for people to mostly create tables
with the default list, so they should generally be in the same order.

Its also probably not a big deal if we end up creating a small number of Kudu clients for
different orderings, its still a significant reduction in the total number created.


http://gerrit.cloudera.org:8080/#/c/6792/5/fe/src/main/java/org/apache/impala/util/KuduUtil.java
File fe/src/main/java/org/apache/impala/util/KuduUtil.java:

Line 72:     b.defaultOperationTimeoutMs(BackendConfig.INSTANCE.getKuduClientTimeoutMs());
> move above the function comment.
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/6792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6b0c12a256c33e8ef32315b3736cae2dea2ae705
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message