HADOOP-13103 Group resolution from LDAP may fail on javax.naming.ServiceUnavailableException
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/f305d9c0 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/f305d9c0 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/f305d9c0 Branch: refs/heads/HDFS-1312 Commit: f305d9c0f64fd7d085f01eaae2154ef13b05b197 Parents: d8faf47 Author: Tsz-Wo Nicholas Sze <szets...@hortonworks.com> Authored: Thu May 5 15:50:50 2016 -0700 Committer: Tsz-Wo Nicholas Sze <szets...@hortonworks.com> Committed: Thu May 5 15:53:02 2016 -0700 ---------------------------------------------------------------------- .../hadoop/security/LdapGroupsMapping.java | 41 +++++++------------- .../hadoop/security/UserGroupInformation.java | 6 ++- .../hadoop/security/TestLdapGroupsMapping.java | 2 +- 3 files changed, 19 insertions(+), 30 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/f305d9c0/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java index 8f6203d..d72aa1e 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java @@ -18,15 +18,14 @@ package org.apache.hadoop.security; import java.io.FileInputStream; -import java.io.FileReader; import java.io.IOException; import java.io.InputStreamReader; import java.io.Reader; import java.util.ArrayList; +import java.util.Collections; import java.util.Hashtable; import java.util.List; -import javax.naming.CommunicationException; import javax.naming.Context; import javax.naming.NamingEnumeration; import javax.naming.NamingException; @@ -43,7 +42,6 @@ import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; import org.apache.hadoop.conf.Configurable; import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.io.IOUtils; /** * An implementation of {@link GroupMappingServiceProvider} which @@ -197,7 +195,7 @@ public class LdapGroupsMapping private String posixGidAttr; private boolean isPosix; - public static int RECONNECT_RETRY_COUNT = 3; + public static final int RECONNECT_RETRY_COUNT = 3; /** * Returns list of groups for a user. @@ -210,40 +208,26 @@ public class LdapGroupsMapping * @return list of groups for a given user */ @Override - public synchronized List<String> getGroups(String user) throws IOException { - List<String> emptyResults = new ArrayList<String>(); + public synchronized List<String> getGroups(String user) { /* * Normal garbage collection takes care of removing Context instances when they are no longer in use. * Connections used by Context instances being garbage collected will be closed automatically. * So in case connection is closed and gets CommunicationException, retry some times with new new DirContext/connection. */ - try { - return doGetGroups(user); - } catch (CommunicationException e) { - LOG.warn("Connection is closed, will try to reconnect"); - } catch (NamingException e) { - LOG.warn("Exception trying to get groups for user " + user + ": " - + e.getMessage()); - return emptyResults; - } - - int retryCount = 0; - while (retryCount ++ < RECONNECT_RETRY_COUNT) { - //reset ctx so that new DirContext can be created with new connection - this.ctx = null; - + for(int retry = 0; retry < RECONNECT_RETRY_COUNT; retry++) { try { return doGetGroups(user); - } catch (CommunicationException e) { - LOG.warn("Connection being closed, reconnecting failed, retryCount = " + retryCount); } catch (NamingException e) { - LOG.warn("Exception trying to get groups for user " + user + ":" - + e.getMessage()); - return emptyResults; + LOG.warn("Failed to get groups for user " + user + " (retry=" + retry + + ") by " + e); + LOG.trace("TRACE", e); } + + //reset ctx so that new DirContext can be created with new connection + this.ctx = null; } - return emptyResults; + return Collections.emptyList(); } List<String> doGetGroups(String user) throws NamingException { @@ -297,6 +281,9 @@ public class LdapGroupsMapping } } + if (LOG.isDebugEnabled()) { + LOG.debug("doGetGroups(" + user + ") return " + groups); + } return groups; } http://git-wip-us.apache.org/repos/asf/hadoop/blob/f305d9c0/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java index 728ac60..798aa01 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java @@ -1613,9 +1613,11 @@ public class UserGroupInformation { return result.toArray(new String[result.size()]); } catch (IOException ie) { if (LOG.isDebugEnabled()) { - LOG.debug("No groups available for user " + getShortUserName()); + LOG.debug("Failed to get groups for user " + getShortUserName() + + " by " + ie); + LOG.trace("TRACE", ie); } - return new String[0]; + return StringUtils.emptyStringArray; } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/f305d9c0/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMapping.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMapping.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMapping.java index da46970..93c81c7 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMapping.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMapping.java @@ -85,7 +85,7 @@ public class TestLdapGroupsMapping extends TestLdapGroupsMappingBase { // Ldap server is down, no groups should be retrieved doTestGetGroups(Arrays.asList(new String[] {}), - 1 + LdapGroupsMapping.RECONNECT_RETRY_COUNT); // 1 is the first normal call + LdapGroupsMapping.RECONNECT_RETRY_COUNT); } private void doTestGetGroups(List<String> expectedGroups, int searchTimes) throws IOException, NamingException { --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
