Alex Behm has posted comments on this change. Change subject: IMPALA-4978: Impala should set the kerberos principal to the FQDN ......................................................................
Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/6260/1/be/src/rpc/authentication.cc File be/src/rpc/authentication.cc: Line 707: if (!GetFQDN(&hostname).ok()) { Looks like there is no way to know from the logs or an error message that we failed to get the FQDN, and that we will fall back to the hostname. Seems like that information should be recorded somewhere for debugging. Line 708: RETURN_IF_ERROR(GetHostname(&hostname)); Pattern seems a little strange since we might end up calling GetHostName() twice, even returning the same error. Alternative to consider: Change GetFQDN() to return a string and a boolean, the latter indicates whether the we got the FQDN or the hostname. http://gerrit.cloudera.org:8080/#/c/6260/1/be/src/util/network-util.cc File be/src/util/network-util.cc: Line 75: string error_msg = GetStrErrMsg(); Do we need to initialize result to NULL and freeaddrinfo() in this error case? I could not find a definitive answer as to whether we need to do this or not in the man pages on getaddrinfo() http://gerrit.cloudera.org:8080/#/c/6260/1/be/src/util/network-util.h File be/src/util/network-util.h: Line 45: // Return the local machine's FQDN. GetFullyQualifiedDomainName()? -- To view, visit http://gerrit.cloudera.org:8080/6260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1c94f36cb3493afdfcf84f8b31b9897404bd095f Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-HasComments: Yes
