Author: shv Date: Tue Jun 5 05:51:22 2012 New Revision: 1346246 URL: http://svn.apache.org/viewvc?rev=1346246&view=rev Log: HADOOP-7115. Add a cache for getpwuid_r and getpwgid_r calls. Contributed by Devaraj Das and Benoy Antony.
Modified: hadoop/common/branches/branch-0.22/common/CHANGES.txt hadoop/common/branches/branch-0.22/common/src/java/core-default.xml hadoop/common/branches/branch-0.22/common/src/java/org/apache/hadoop/io/SecureIOUtils.java hadoop/common/branches/branch-0.22/common/src/java/org/apache/hadoop/io/nativeio/NativeIO.java hadoop/common/branches/branch-0.22/common/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c hadoop/common/branches/branch-0.22/common/src/native/src/org/apache/hadoop/security/getGroup.c hadoop/common/branches/branch-0.22/common/src/test/core/org/apache/hadoop/io/TestSecureIOUtils.java hadoop/common/branches/branch-0.22/common/src/test/core/org/apache/hadoop/io/nativeio/TestNativeIO.java Modified: hadoop/common/branches/branch-0.22/common/CHANGES.txt URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/common/CHANGES.txt?rev=1346246&r1=1346245&r2=1346246&view=diff ============================================================================== --- hadoop/common/branches/branch-0.22/common/CHANGES.txt (original) +++ hadoop/common/branches/branch-0.22/common/CHANGES.txt Tue Jun 5 05:51:22 2012 @@ -44,6 +44,9 @@ Release 0.22.1 - Unreleased the host in the client's kerberos principal key. (Suresh Srinivas and Benoy Antony via shv) + HADOOP-7115. Add a cache for getpwuid_r and getpwgid_r calls. + (Devaraj Das and Benoy Antony via shv) + Release 0.22.0 - 2011-11-29 INCOMPATIBLE CHANGES Modified: hadoop/common/branches/branch-0.22/common/src/java/core-default.xml URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/common/src/java/core-default.xml?rev=1346246&r1=1346245&r2=1346246&view=diff ============================================================================== --- hadoop/common/branches/branch-0.22/common/src/java/core-default.xml (original) +++ hadoop/common/branches/branch-0.22/common/src/java/core-default.xml Tue Jun 5 05:51:22 2012 @@ -520,6 +520,12 @@ </description> </property> +<property> + <name>hadoop.security.uid.cache.secs</name> + <value>14400</value> + <description> NativeIO maintains a cache from UID to UserName. This is + the timeout for an entry in that cache. </description> +</property> <property> Modified: hadoop/common/branches/branch-0.22/common/src/java/org/apache/hadoop/io/SecureIOUtils.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/common/src/java/org/apache/hadoop/io/SecureIOUtils.java?rev=1346246&r1=1346245&r2=1346246&view=diff ============================================================================== --- hadoop/common/branches/branch-0.22/common/src/java/org/apache/hadoop/io/SecureIOUtils.java (original) +++ hadoop/common/branches/branch-0.22/common/src/java/org/apache/hadoop/io/SecureIOUtils.java Tue Jun 5 05:51:22 2012 @@ -24,14 +24,12 @@ import java.io.FileOutputStream; import java.io.IOException; import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.io.nativeio.Errno; import org.apache.hadoop.io.nativeio.NativeIO; import org.apache.hadoop.io.nativeio.NativeIOException; -import org.apache.hadoop.io.nativeio.NativeIO.Stat; import org.apache.hadoop.security.UserGroupInformation; /** @@ -90,7 +88,7 @@ public class SecureIOUtils { private final static FileSystem rawFilesystem; /** - * Open the given File for read access, verifying the expected user/group + * Open the given File for read access, verifying the expected user * constraints if security is enabled. * * Note that this function provides no additional checks if Hadoop @@ -98,32 +96,30 @@ public class SecureIOUtils { * when native libraries are not available. * * @param f the file that we are trying to open - * @param expectedOwner the expected user owner for the file - * @param expectedGroup the expected group owner for the file + * @param expectedOwner the expected user owner for the file * @throws IOException if an IO Error occurred, or security is enabled and - * the user/group does not match + * the user does not match */ - public static FileInputStream openForRead(File f, String expectedOwner, - String expectedGroup) throws IOException { + public static FileInputStream openForRead(File f, String expectedOwner) + throws IOException { if (!UserGroupInformation.isSecurityEnabled()) { return new FileInputStream(f); } - return forceSecureOpenForRead(f, expectedOwner, expectedGroup); + return forceSecureOpenForRead(f, expectedOwner); } /** * Same as openForRead() except that it will run even if security is off. * This is used by unit tests. */ - static FileInputStream forceSecureOpenForRead(File f, String expectedOwner, - String expectedGroup) throws IOException { + static FileInputStream forceSecureOpenForRead(File f, String expectedOwner) + throws IOException { FileInputStream fis = new FileInputStream(f); boolean success = false; try { - Stat stat = NativeIO.fstat(fis.getFD()); - checkStat(f, stat.getOwner(), stat.getGroup(), expectedOwner, - expectedGroup); + String owner = NativeIO.getOwner(fis.getFD()); + checkStat(f, owner, expectedOwner); success = true; return fis; } finally { @@ -182,21 +178,13 @@ public class SecureIOUtils { } } - private static void checkStat(File f, String owner, String group, - String expectedOwner, - String expectedGroup) throws IOException { + private static void checkStat(File f, String owner, String expectedOwner) throws IOException { if (expectedOwner != null && !expectedOwner.equals(owner)) { throw new IOException( "Owner '" + owner + "' for path " + f + " did not match " + "expected owner '" + expectedOwner + "'"); } - if (expectedGroup != null && - !expectedGroup.equals(group)) { - throw new IOException( - "Group '" + group + "' for path " + f + " did not match " + - "expected group '" + expectedGroup + "'"); - } } /** Modified: hadoop/common/branches/branch-0.22/common/src/java/org/apache/hadoop/io/nativeio/NativeIO.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/common/src/java/org/apache/hadoop/io/nativeio/NativeIO.java?rev=1346246&r1=1346245&r2=1346246&view=diff ============================================================================== --- hadoop/common/branches/branch-0.22/common/src/java/org/apache/hadoop/io/nativeio/NativeIO.java (original) +++ hadoop/common/branches/branch-0.22/common/src/java/org/apache/hadoop/io/nativeio/NativeIO.java Tue Jun 5 05:51:22 2012 @@ -19,6 +19,8 @@ package org.apache.hadoop.io.nativeio; import java.io.FileDescriptor; import java.io.IOException; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.util.NativeCodeLoader; @@ -84,19 +86,63 @@ public class NativeIO { /** Wrapper around open(2) */ public static native FileDescriptor open(String path, int flags, int mode) throws IOException; /** Wrapper around fstat(2) */ + //TODO: fstat is an old implementation. Doesn't use the cache. This should be + //changed to use the cache. public static native Stat fstat(FileDescriptor fd) throws IOException; + private static native long getUIDforFDOwnerforOwner(FileDescriptor fd) throws IOException; + private static native String getUserName(long uid) throws IOException; /** Wrapper around chmod(2) */ public static native void chmod(String path, int mode) throws IOException; /** Initialize the JNI method ID and class ID cache */ private static native void initNative(); + + private static class CachedUid { + final long timestamp; + final String username; + public CachedUid(String username, long timestamp) { + this.timestamp = timestamp; + this.username = username; + } + } + private static final Map<Long, CachedUid> uidCache = + new ConcurrentHashMap<Long, CachedUid>(); + private static long cacheTimeout; + private static boolean initialized = false; + + public static String getOwner(FileDescriptor fd) throws IOException { + ensureInitialized(); + long uid = getUIDforFDOwnerforOwner(fd); + CachedUid cUid = uidCache.get(uid); + long now = System.currentTimeMillis(); + if (cUid != null && (cUid.timestamp + cacheTimeout) > now) { + return cUid.username; + } + String user = getUserName(uid); + LOG.info("Got UserName " + user + " for UID " + uid + + " from the native implementation"); + cUid = new CachedUid(user, now); + uidCache.put(uid, cUid); + return user; + } + + private synchronized static void ensureInitialized() { + if (!initialized) { + cacheTimeout = + new Configuration().getLong("hadoop.security.uid.cache.secs", + 4*60*60) * 1000; + LOG.info("Initialized cache for UID to User mapping with a cache" + + " timeout of " + cacheTimeout/1000 + " seconds."); + initialized = true; + } + } /** * Result type of the fstat call */ public static class Stat { - private String owner, group; + private String owner; private int mode; // Mode constants @@ -116,23 +162,19 @@ public class NativeIO { public static final int S_IWUSR = 0000200; /* write permission, owner */ public static final int S_IXUSR = 0000100; /* execute/search permission, owner */ - Stat(String owner, String group, int mode) { + Stat(String owner, int mode) { this.owner = owner; - this.group = group; this.mode = mode; } public String toString() { - return "Stat(owner='" + owner + "', group='" + group + "'" + + return "Stat(owner='" + owner + "'" + ", mode=" + mode + ")"; } public String getOwner() { return owner; } - public String getGroup() { - return group; - } public int getMode() { return mode; } Modified: hadoop/common/branches/branch-0.22/common/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/common/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c?rev=1346246&r1=1346245&r2=1346246&view=diff ============================================================================== --- hadoop/common/branches/branch-0.22/common/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c (original) +++ hadoop/common/branches/branch-0.22/common/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c Tue Jun 5 05:51:22 2012 @@ -74,7 +74,7 @@ static void stat_init(JNIEnv *env, jclas PASS_EXCEPTIONS(env); stat_clazz = (*env)->NewGlobalRef(env, clazz); stat_ctor = (*env)->GetMethodID(env, stat_clazz, "<init>", - "(Ljava/lang/String;Ljava/lang/String;I)V"); + "(Ljava/lang/String;I)V"); jclass obj_class = (*env)->FindClass(env, "java/lang/Object"); assert(obj_class != NULL); @@ -197,33 +197,17 @@ Java_org_apache_hadoop_io_nativeio_Nativ goto cleanup; } } - assert(pwdp == &pwd); + if (rc == 0 && pwdp == NULL) { + throw_ioe(env, ENOENT); + goto cleanup; + } jstring jstr_username = (*env)->NewStringUTF(env, pwd.pw_name); if (jstr_username == NULL) goto cleanup; - // Grab group - struct group grp, *grpp; - while ((rc = getgrgid_r(s.st_gid, &grp, pw_buf, pw_buflen, &grpp)) != 0) { - if (rc != ERANGE) { - throw_ioe(env, rc); - goto cleanup; - } - free(pw_buf); - pw_buflen *= 2; - if ((pw_buf = malloc(pw_buflen)) == NULL) { - THROW(env, "java/lang/OutOfMemoryError", "Couldn't allocate memory for pw buffer"); - goto cleanup; - } - } - assert(grpp == &grp); - - jstring jstr_groupname = (*env)->NewStringUTF(env, grp.gr_name); - PASS_EXCEPTIONS_GOTO(env, cleanup); - // Construct result ret = (*env)->NewObject(env, stat_clazz, stat_ctor, - jstr_username, jstr_groupname, s.st_mode); + jstr_username, s.st_mode); cleanup: if (pw_buf != NULL) free(pw_buf); @@ -288,6 +272,67 @@ Java_org_apache_hadoop_io_nativeio_Nativ /* + * private static native long getUIDforFDOwnerforOwner(FileDescriptor fd); + */ +JNIEXPORT jlong JNICALL +Java_org_apache_hadoop_io_nativeio_NativeIO_getUIDforFDOwnerforOwner(JNIEnv *env, jclass clazz, + jobject fd_object) { + int fd = fd_get(env, fd_object); + PASS_EXCEPTIONS_GOTO(env, cleanup); + + struct stat s; + int rc = fstat(fd, &s); + if (rc != 0) { + throw_ioe(env, errno); + goto cleanup; + } + return (jlong)(s.st_uid); +cleanup: + return (jlong)(-1); +} + +/* + * private static native String getUserName(long uid); + */ +JNIEXPORT jstring JNICALL +Java_org_apache_hadoop_io_nativeio_NativeIO_getUserName(JNIEnv *env, +jclass clazz, jlong uid) { + + char *pw_buf = NULL; + int rc; + size_t pw_buflen = get_pw_buflen(); + if ((pw_buf = malloc(pw_buflen)) == NULL) { + THROW(env, "java/lang/OutOfMemoryError", "Couldn't allocate memory for pw buffer"); + goto cleanup; + } + + // Grab username + struct passwd pwd, *pwdp; + while ((rc = getpwuid_r((uid_t)uid, &pwd, pw_buf, pw_buflen, &pwdp)) != 0) { + if (rc != ERANGE) { + throw_ioe(env, rc); + goto cleanup; + } + free(pw_buf); + pw_buflen *= 2; + if ((pw_buf = malloc(pw_buflen)) == NULL) { + THROW(env, "java/lang/OutOfMemoryError", "Couldn't allocate memory for pw buffer"); + goto cleanup; + } + } + if (rc == 0 && pwdp == NULL) { + throw_ioe(env, ENOENT); + goto cleanup; + } + + jstring jstr_username = (*env)->NewStringUTF(env, pwd.pw_name); + +cleanup: + if (pw_buf != NULL) free(pw_buf); + return jstr_username; +} + +/* * Throw a java.IO.IOException, generating the message from errno. */ static void throw_ioe(JNIEnv* env, int errnum) Modified: hadoop/common/branches/branch-0.22/common/src/native/src/org/apache/hadoop/security/getGroup.c URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/common/src/native/src/org/apache/hadoop/security/getGroup.c?rev=1346246&r1=1346245&r2=1346246&view=diff ============================================================================== --- hadoop/common/branches/branch-0.22/common/src/native/src/org/apache/hadoop/security/getGroup.c (original) +++ hadoop/common/branches/branch-0.22/common/src/native/src/org/apache/hadoop/security/getGroup.c Tue Jun 5 05:51:22 2012 @@ -23,6 +23,7 @@ #include <string.h> #include <stdlib.h> +#define MAX(a, b) (a > b ? a : b) /*Helper functions for the JNI implementation of unix group mapping service*/ @@ -78,7 +79,7 @@ int getGroupIDList(const char *user, int */ int getGroupDetails(gid_t group, char **grpBuf) { struct group * grp = NULL; - size_t currBufferSize = sysconf(_SC_GETGR_R_SIZE_MAX); + size_t currBufferSize = MAX(sysconf(_SC_GETGR_R_SIZE_MAX), 2048); if (currBufferSize < 1024) { currBufferSize = 1024; } @@ -123,7 +124,7 @@ int getGroupDetails(gid_t group, char ** */ int getPW(const char *user, char **pwbuf) { struct passwd *pwbufp = NULL; - size_t currBufferSize = sysconf(_SC_GETPW_R_SIZE_MAX); + size_t currBufferSize = MAX(sysconf(_SC_GETPW_R_SIZE_MAX), 2048); if (currBufferSize < 1024) { currBufferSize = 1024; } Modified: hadoop/common/branches/branch-0.22/common/src/test/core/org/apache/hadoop/io/TestSecureIOUtils.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/common/src/test/core/org/apache/hadoop/io/TestSecureIOUtils.java?rev=1346246&r1=1346245&r2=1346246&view=diff ============================================================================== --- hadoop/common/branches/branch-0.22/common/src/test/core/org/apache/hadoop/io/TestSecureIOUtils.java (original) +++ hadoop/common/branches/branch-0.22/common/src/test/core/org/apache/hadoop/io/TestSecureIOUtils.java Tue Jun 5 05:51:22 2012 @@ -35,7 +35,7 @@ import java.io.FileInputStream; import java.io.FileOutputStream; public class TestSecureIOUtils { - private static String realOwner, realGroup; + private static String realOwner; private static final File testFilePath = new File(System.getProperty("test.build.data"), "TestSecureIOContext"); @@ -50,18 +50,17 @@ public class TestSecureIOUtils { FileStatus stat = rawFS.getFileStatus( new Path(testFilePath.toString())); realOwner = stat.getOwner(); - realGroup = stat.getGroup(); } @Test public void testReadUnrestricted() throws IOException { - SecureIOUtils.openForRead(testFilePath, null, null).close(); + SecureIOUtils.openForRead(testFilePath, null).close(); } @Test public void testReadCorrectlyRestrictedWithSecurity() throws IOException { SecureIOUtils - .openForRead(testFilePath, realOwner, realGroup).close(); + .openForRead(testFilePath, realOwner).close(); } @Test @@ -73,7 +72,7 @@ public class TestSecureIOUtils { try { SecureIOUtils - .forceSecureOpenForRead(testFilePath, "invalidUser", null).close(); + .forceSecureOpenForRead(testFilePath, "invalidUser").close(); fail("Didn't throw expection for wrong ownership!"); } catch (IOException ioe) { // expected Modified: hadoop/common/branches/branch-0.22/common/src/test/core/org/apache/hadoop/io/nativeio/TestNativeIO.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/common/src/test/core/org/apache/hadoop/io/nativeio/TestNativeIO.java?rev=1346246&r1=1346245&r2=1346246&view=diff ============================================================================== --- hadoop/common/branches/branch-0.22/common/src/test/core/org/apache/hadoop/io/nativeio/TestNativeIO.java (original) +++ hadoop/common/branches/branch-0.22/common/src/test/core/org/apache/hadoop/io/nativeio/TestNativeIO.java Tue Jun 5 05:51:22 2012 @@ -64,11 +64,21 @@ public class TestNativeIO { LOG.info("Stat: " + String.valueOf(stat)); assertEquals(System.getProperty("user.name"), stat.getOwner()); - assertNotNull(stat.getGroup()); - assertTrue(!"".equals(stat.getGroup())); assertEquals("Stat mode field should indicate a regular file", NativeIO.Stat.S_IFREG, stat.getMode() & NativeIO.Stat.S_IFMT); } + + @Test + public void testGetOwner() throws Exception { + FileOutputStream fos = new FileOutputStream( + new File(TEST_DIR, "testfstat")); + String owner = NativeIO.getOwner(fos.getFD()); + fos.close(); + LOG.info("Owner: " + owner); + + assertEquals(System.getProperty("user.name"), owner); + } + /** * Test for races in fstat usage @@ -92,8 +102,6 @@ public class TestNativeIO { try { NativeIO.Stat stat = NativeIO.fstat(fos.getFD()); assertEquals(System.getProperty("user.name"), stat.getOwner()); - assertNotNull(stat.getGroup()); - assertTrue(!"".equals(stat.getGroup())); assertEquals("Stat mode field should indicate a regular file", NativeIO.Stat.S_IFREG, stat.getMode() & NativeIO.Stat.S_IFMT); } catch (Throwable t) {