impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Tauber-Marshall (Code Review)" <>
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:


> 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?
File be/src/runtime/

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

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.
File fe/src/main/java/org/apache/impala/util/

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

To view, visit
To unsubscribe, visit

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

View raw message