HADOOP-13442. Optimize UGI group lookups. Contributed by Daryn Sharp.
(cherry picked from commit 9422515239
)
This commit is contained in:
parent
58df27b873
commit
1b97519987
|
@ -26,7 +26,6 @@ import java.net.URI;
|
|||
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;
|
||||
|
@ -2217,12 +2216,11 @@ public abstract class FileSystem extends Configured implements Closeable {
|
|||
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;
|
||||
}
|
||||
|
|
|
@ -862,7 +862,7 @@ public class ViewFileSystem extends FileSystem {
|
|||
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));
|
||||
|
@ -883,7 +883,7 @@ public class ViewFileSystem extends FileSystem {
|
|||
|
||||
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));
|
||||
|
@ -1015,7 +1015,7 @@ public class ViewFileSystem extends FileSystem {
|
|||
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();
|
||||
}
|
||||
|
|
|
@ -843,14 +843,14 @@ public class ViewFs extends AbstractFileSystem {
|
|||
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 class ViewFs extends AbstractFileSystem {
|
|||
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 class ViewFs extends AbstractFileSystem {
|
|||
|
||||
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 class ViewFs extends AbstractFileSystem {
|
|||
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();
|
||||
}
|
||||
|
|
|
@ -23,7 +23,6 @@ import java.io.FileInputStream;
|
|||
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;
|
||||
|
@ -276,7 +275,7 @@ public class SecureIOUtils {
|
|||
UserGroupInformation.createRemoteUser(expectedOwner);
|
||||
final String adminsGroupString = "Administrators";
|
||||
success = owner.equals(adminsGroupString)
|
||||
&& Arrays.asList(ugi.getGroupNames()).contains(adminsGroupString);
|
||||
&& ugi.getGroups().contains(adminsGroupString);
|
||||
} else {
|
||||
success = false;
|
||||
}
|
||||
|
|
|
@ -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.ThreadFactory;
|
|||
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 @@ public class Groups {
|
|||
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 @@ public class Groups {
|
|||
}
|
||||
staticUserToGroupsMap.put(user, groups);
|
||||
}
|
||||
staticMapRef.set(
|
||||
staticUserToGroupsMap.isEmpty() ? null : staticUserToGroupsMap);
|
||||
}
|
||||
|
||||
private boolean isNegativeCacheEnabled() {
|
||||
|
@ -201,9 +208,12 @@ public class Groups {
|
|||
*/
|
||||
public List<String> getGroups(final String user) throws IOException {
|
||||
// No need to lookup for groups of static users
|
||||
List<String> staticMapping = staticUserToGroupsMap.get(user);
|
||||
if (staticMapping != null) {
|
||||
return staticMapping;
|
||||
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
|
||||
|
@ -322,7 +332,9 @@ public class Groups {
|
|||
throw noGroupsForUser(user);
|
||||
}
|
||||
|
||||
return groups;
|
||||
// return immutable de-duped list
|
||||
return Collections.unmodifiableList(
|
||||
new ArrayList<>(new LinkedHashSet<>(groups)));
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -37,7 +37,6 @@ import java.util.Collections;
|
|||
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;
|
||||
|
@ -1467,11 +1466,11 @@ public class UserGroupInformation {
|
|||
}
|
||||
|
||||
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);
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -1583,27 +1582,36 @@ public class UserGroupInformation {
|
|||
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();
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Return the username.
|
||||
*/
|
||||
|
|
|
@ -231,7 +231,7 @@ public class AccessControlList implements Writable {
|
|||
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;
|
||||
}
|
||||
|
|
|
@ -441,8 +441,10 @@ public class TestUserGroupInformation {
|
|||
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
|
||||
|
|
Loading…
Reference in New Issue