HADOOP-13442. Optimize UGI group lookups. Contributed by Daryn Sharp.

This commit is contained in:
Kihwal Lee 2016-08-04 10:45:55 -05:00
parent 6ae39199da
commit 9422515239
8 changed files with 52 additions and 33 deletions

View File

@ -26,7 +26,6 @@
import java.net.URISyntaxException;
import java.security.PrivilegedExceptionAction;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.EnumSet;
import java.util.HashMap;
@ -2221,12 +2220,11 @@ static void checkAccessPermissions(FileStatus stat, FsAction mode)
FsPermission perm = stat.getPermission();
UserGroupInformation ugi = UserGroupInformation.getCurrentUser();
String user = ugi.getShortUserName();
List<String> groups = Arrays.asList(ugi.getGroupNames());
if (user.equals(stat.getOwner())) {
if (perm.getUserAction().implies(mode)) {
return;
}
} else if (groups.contains(stat.getGroup())) {
} else if (ugi.getGroups().contains(stat.getGroup())) {
if (perm.getGroupAction().implies(mode)) {
return;
}

View File

@ -901,7 +901,7 @@ public FileChecksum getFileChecksum(final Path f)
public FileStatus getFileStatus(Path f) throws IOException {
checkPathIsSlash(f);
return new FileStatus(0, true, 0, 0, creationTime, creationTime,
PERMISSION_555, ugi.getUserName(), ugi.getGroupNames()[0],
PERMISSION_555, ugi.getUserName(), ugi.getPrimaryGroupName(),
new Path(theInternalDir.fullPath).makeQualified(
myUri, ROOT_PATH));
@ -922,7 +922,7 @@ public FileStatus[] listStatus(Path f) throws AccessControlException,
result[i++] = new FileStatus(0, false, 0, 0,
creationTime, creationTime, PERMISSION_555,
ugi.getUserName(), ugi.getGroupNames()[0],
ugi.getUserName(), ugi.getPrimaryGroupName(),
link.getTargetLink(),
new Path(inode.fullPath).makeQualified(
myUri, null));
@ -1054,7 +1054,7 @@ public void setAcl(Path path, List<AclEntry> aclSpec) throws IOException {
public AclStatus getAclStatus(Path path) throws IOException {
checkPathIsSlash(path);
return new AclStatus.Builder().owner(ugi.getUserName())
.group(ugi.getGroupNames()[0])
.group(ugi.getPrimaryGroupName())
.addEntries(AclUtil.getMinimalAcl(PERMISSION_555))
.stickyBit(false).build();
}

View File

@ -843,14 +843,14 @@ public FileChecksum getFileChecksum(final Path f)
public FileStatus getFileStatus(final Path f) throws IOException {
checkPathIsSlash(f);
return new FileStatus(0, true, 0, 0, creationTime, creationTime,
PERMISSION_555, ugi.getUserName(), ugi.getGroupNames()[0],
PERMISSION_555, ugi.getUserName(), ugi.getPrimaryGroupName(),
new Path(theInternalDir.fullPath).makeQualified(
myUri, null));
}
@Override
public FileStatus getFileLinkStatus(final Path f)
throws FileNotFoundException {
throws IOException {
// look up i internalDirs children - ignore first Slash
INode<AbstractFileSystem> inode =
theInternalDir.children.get(f.toUri().toString().substring(1));
@ -863,13 +863,13 @@ public FileStatus getFileLinkStatus(final Path f)
INodeLink<AbstractFileSystem> inodelink =
(INodeLink<AbstractFileSystem>) inode;
result = new FileStatus(0, false, 0, 0, creationTime, creationTime,
PERMISSION_555, ugi.getUserName(), ugi.getGroupNames()[0],
PERMISSION_555, ugi.getUserName(), ugi.getPrimaryGroupName(),
inodelink.getTargetLink(),
new Path(inode.fullPath).makeQualified(
myUri, null));
} else {
result = new FileStatus(0, true, 0, 0, creationTime, creationTime,
PERMISSION_555, ugi.getUserName(), ugi.getGroupNames()[0],
PERMISSION_555, ugi.getUserName(), ugi.getPrimaryGroupName(),
new Path(inode.fullPath).makeQualified(
myUri, null));
}
@ -908,7 +908,7 @@ public FileStatus[] listStatus(final Path f) throws AccessControlException,
result[i++] = new FileStatus(0, false, 0, 0,
creationTime, creationTime,
PERMISSION_555, ugi.getUserName(), ugi.getGroupNames()[0],
PERMISSION_555, ugi.getUserName(), ugi.getPrimaryGroupName(),
link.getTargetLink(),
new Path(inode.fullPath).makeQualified(
myUri, null));
@ -1042,7 +1042,7 @@ public void setAcl(Path path, List<AclEntry> aclSpec) throws IOException {
public AclStatus getAclStatus(Path path) throws IOException {
checkPathIsSlash(path);
return new AclStatus.Builder().owner(ugi.getUserName())
.group(ugi.getGroupNames()[0])
.group(ugi.getPrimaryGroupName())
.addEntries(AclUtil.getMinimalAcl(PERMISSION_555))
.stickyBit(false).build();
}

View File

@ -22,7 +22,6 @@
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.RandomAccessFile;
import java.util.Arrays;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FSDataInputStream;
@ -273,7 +272,7 @@ private static void checkStat(File f, String owner, String group,
UserGroupInformation.createRemoteUser(expectedOwner);
final String adminsGroupString = "Administrators";
success = owner.equals(adminsGroupString)
&& Arrays.asList(ugi.getGroupNames()).contains(adminsGroupString);
&& ugi.getGroups().contains(adminsGroupString);
} else {
success = false;
}

View File

@ -18,9 +18,11 @@
package org.apache.hadoop.security;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@ -31,6 +33,7 @@
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
import org.apache.htrace.core.TraceScope;
import org.apache.htrace.core.Tracer;
@ -74,8 +77,8 @@ public class Groups {
private final GroupMappingServiceProvider impl;
private final LoadingCache<String, List<String>> cache;
private final Map<String, List<String>> staticUserToGroupsMap =
new HashMap<String, List<String>>();
private final AtomicReference<Map<String, List<String>>> staticMapRef =
new AtomicReference<>();
private final long cacheTimeout;
private final long negativeCacheTimeout;
private final long warningDeltaMs;
@ -164,6 +167,8 @@ private void parseStaticMapping(Configuration conf) {
CommonConfigurationKeys.HADOOP_USER_GROUP_STATIC_OVERRIDES_DEFAULT);
Collection<String> mappings = StringUtils.getStringCollection(
staticMapping, ";");
Map<String, List<String>> staticUserToGroupsMap =
new HashMap<String, List<String>>();
for (String users : mappings) {
Collection<String> userToGroups = StringUtils.getStringCollection(users,
"=");
@ -182,6 +187,8 @@ private void parseStaticMapping(Configuration conf) {
}
staticUserToGroupsMap.put(user, groups);
}
staticMapRef.set(
staticUserToGroupsMap.isEmpty() ? null : staticUserToGroupsMap);
}
private boolean isNegativeCacheEnabled() {
@ -201,10 +208,13 @@ private IOException noGroupsForUser(String user) {
*/
public List<String> getGroups(final String user) throws IOException {
// No need to lookup for groups of static users
Map<String, List<String>> staticUserToGroupsMap = staticMapRef.get();
if (staticUserToGroupsMap != null) {
List<String> staticMapping = staticUserToGroupsMap.get(user);
if (staticMapping != null) {
return staticMapping;
}
}
// Check the negative cache first
if (isNegativeCacheEnabled()) {
@ -322,7 +332,9 @@ public List<String> load(String user) throws Exception {
throw noGroupsForUser(user);
}
return groups;
// return immutable de-duped list
return Collections.unmodifiableList(
new ArrayList<>(new LinkedHashSet<>(groups)));
}
/**

View File

@ -40,7 +40,6 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@ -1511,11 +1510,11 @@ public String getShortUserName() {
}
public String getPrimaryGroupName() throws IOException {
String[] groups = getGroupNames();
if (groups.length == 0) {
List<String> groups = getGroups();
if (groups.isEmpty()) {
throw new IOException("There is no primary group for UGI " + this);
}
return groups[0];
return groups.get(0);
}
/**
@ -1627,24 +1626,33 @@ private synchronized Credentials getCredentialsInternal() {
return credentials;
}
/**
* Get the group names for this user. {@ #getGroups(String)} is less
* expensive alternative when checking for a contained element.
* @return the list of users with the primary group first. If the command
* fails, it returns an empty list.
*/
public String[] getGroupNames() {
List<String> groups = getGroups();
return groups.toArray(new String[groups.size()]);
}
/**
* Get the group names for this user.
* @return the list of users with the primary group first. If the command
* fails, it returns an empty list.
*/
public synchronized String[] getGroupNames() {
public List<String> getGroups() {
ensureInitialized();
try {
Set<String> result = new LinkedHashSet<String>
(groups.getGroups(getShortUserName()));
return result.toArray(new String[result.size()]);
return groups.getGroups(getShortUserName());
} catch (IOException ie) {
if (LOG.isDebugEnabled()) {
LOG.debug("Failed to get groups for user " + getShortUserName()
+ " by " + ie);
LOG.trace("TRACE", ie);
}
return StringUtils.emptyStringArray;
return Collections.emptyList();
}
}

View File

@ -231,7 +231,7 @@ public final boolean isUserInList(UserGroupInformation ugi) {
if (allAllowed || users.contains(ugi.getShortUserName())) {
return true;
} else if (!groups.isEmpty()) {
for(String group: ugi.getGroupNames()) {
for (String group : ugi.getGroups()) {
if (groups.contains(group)) {
return true;
}

View File

@ -444,8 +444,10 @@ public void testGettingGroups() throws Exception {
UserGroupInformation uugi =
UserGroupInformation.createUserForTesting(USER_NAME, GROUP_NAMES);
assertEquals(USER_NAME, uugi.getUserName());
assertArrayEquals(new String[]{GROUP1_NAME, GROUP2_NAME, GROUP3_NAME},
uugi.getGroupNames());
String[] expected = new String[]{GROUP1_NAME, GROUP2_NAME, GROUP3_NAME};
assertArrayEquals(expected, uugi.getGroupNames());
assertArrayEquals(expected, uugi.getGroups().toArray(new String[0]));
assertEquals(GROUP1_NAME, uugi.getPrimaryGroupName());
}
@SuppressWarnings("unchecked") // from Mockito mocks