HADOOP-7115. Add a cache for getpwuid_r and getpwgid_r calls. Contributed by Devaraj Das and Benoy Antony.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-0.22@1346246 13f79535-47bb-0310-9956-ffa450edef68

common/CHANGES.txt

@@ -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

common/src/java/core-default.xml

@@ -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>

common/src/java/org/apache/hadoop/io/SecureIOUtils.java

@@ -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 + "'");
-    }
   }
 
   /**

common/src/java/org/apache/hadoop/io/nativeio/NativeIO.java

@@ -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;
     }

common/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c

@@ -74,7 +74,7 @@ static void stat_init(JNIEnv *env, jclass nativeio_class) {
   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_NativeIO_fstat(
       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);
@@ -287,6 +271,67 @@ Java_org_apache_hadoop_io_nativeio_NativeIO_chmod(
 }
 
 
+/*
+ * 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.
  */

common/src/native/src/org/apache/hadoop/security/getGroup.c

@@ -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 *ngroups, gid_t **groups) {
  */
 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 **grpBuf) {
  */
 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;
   }

common/src/test/core/org/apache/hadoop/io/TestSecureIOUtils.java

@@ -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

common/src/test/core/org/apache/hadoop/io/nativeio/TestNativeIO.java

@@ -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) {