impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4978: Impala should set the kerberos principal to the FQDN
Date Tue, 18 Apr 2017 22:14:04 GMT
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 <sailesh@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message