From f305d9c0f64fd7d085f01eaae2154ef13b05b197 Mon Sep 17 00:00:00 2001 From: Tsz-Wo Nicholas Sze Date: Thu, 5 May 2016 15:50:50 -0700 Subject: [PATCH] HADOOP-13103 Group resolution from LDAP may fail on javax.naming.ServiceUnavailableException --- .../hadoop/security/LdapGroupsMapping.java | 41 +++++++------------ .../hadoop/security/UserGroupInformation.java | 6 ++- .../security/TestLdapGroupsMapping.java | 2 +- 3 files changed, 19 insertions(+), 30 deletions(-) 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 8f6203de45..d72aa1e414 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.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 getGroups(String user) throws IOException { - List emptyResults = new ArrayList(); + public synchronized List 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 doGetGroups(String user) throws NamingException { @@ -297,6 +281,9 @@ List doGetGroups(String user) throws NamingException { } } + if (LOG.isDebugEnabled()) { + LOG.debug("doGetGroups(" + user + ") return " + groups); + } return groups; } 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 728ac60a01..798aa01f29 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 synchronized String[] getGroupNames() { 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; } } 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 da46970270..93c81c774c 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 void testGetGroupsWithLdapDown() throws IOException, NamingException { // 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 expectedGroups, int searchTimes) throws IOException, NamingException {