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 <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-HasComments: Yes
