hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From apurt...@apache.org
Subject [1/2] hbase git commit: HBASE-14512 Cache UGI groups
Date Sat, 14 Nov 2015 00:45:02 GMT
Repository: hbase
Updated Branches:
  refs/heads/0.98 0b9602937 -> d29f68dc1


HBASE-14512 Cache UGI groups

Summary:
Don't create a user for every call. Instead create one user per connection.
Then inside of SecureHadoopUser cache the groupNames. This allows HBase to cache
empty groups.  This is needed since it's much more likely that HBase will be accessed
by a user that's not present on the posix /etc/group file.

Test Plan: Unit Tests

Differential Revision: https://reviews.facebook.net/D47751

Conflicts:
	hbase-common/src/main/java/org/apache/hadoop/hbase/security/User.java
	hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/CallRunner.java
	hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/8d947730
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/8d947730
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/8d947730

Branch: refs/heads/0.98
Commit: 8d94773044a68ba601eecd4a08b02c48df5cbaa7
Parents: 0b96029
Author: Elliott Clark <eclark@apache.org>
Authored: Tue Sep 29 14:17:24 2015 -0700
Committer: Andrew Purtell <apurtell@apache.org>
Committed: Fri Nov 13 14:20:54 2015 -0800

----------------------------------------------------------------------
 .../org/apache/hadoop/hbase/security/User.java  | 35 +++++++--
 .../hadoop/hbase/security/UserProvider.java     | 60 ++++++++++++++-
 .../org/apache/hadoop/hbase/ipc/CallRunner.java |  7 +-
 .../org/apache/hadoop/hbase/ipc/RpcServer.java  | 79 ++++++++++----------
 4 files changed, 131 insertions(+), 50 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/8d947730/hbase-common/src/main/java/org/apache/hadoop/hbase/security/User.java
----------------------------------------------------------------------
diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/security/User.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/security/User.java
index c4d6af4..d017cf3 100644
--- a/hbase-common/src/main/java/org/apache/hadoop/hbase/security/User.java
+++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/security/User.java
@@ -24,9 +24,10 @@ import java.lang.reflect.UndeclaredThrowableException;
 import java.security.PrivilegedAction;
 import java.security.PrivilegedExceptionAction;
 import java.util.Collection;
+import java.util.concurrent.ExecutionException;
+
+import com.google.common.cache.LoadingCache;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.classification.InterfaceAudience;
 import org.apache.hadoop.hbase.classification.InterfaceStability;
@@ -57,8 +58,6 @@ public abstract class User {
   public static final String HBASE_SECURITY_AUTHORIZATION_CONF_KEY =
       "hbase.security.authorization";
 
-  private static Log LOG = LogFactory.getLog(User.class);
-
   protected UserGroupInformation ugi;
 
   public UserGroupInformation getUGI() {
@@ -285,15 +284,25 @@ public abstract class User {
    * {@link org.apache.hadoop.security.UserGroupInformation} for secure Hadoop
    * 0.20 and versions 0.21 and above.
    */
-  private static class SecureHadoopUser extends User {
+  @InterfaceAudience.Private
+   public static final class SecureHadoopUser extends User {
     private String shortName;
+    private LoadingCache<UserGroupInformation, String[]> cache;
 
-    private SecureHadoopUser() throws IOException {
+    public SecureHadoopUser() throws IOException {
       ugi = UserGroupInformation.getCurrentUser();
+      this.cache = null;
+    }
+
+    public SecureHadoopUser(UserGroupInformation ugi) {
+      this.ugi = ugi;
+      this.cache = null;
     }
 
-    private SecureHadoopUser(UserGroupInformation ugi) {
+    public SecureHadoopUser(UserGroupInformation ugi,
+                            LoadingCache<UserGroupInformation, String[]> cache) {
       this.ugi = ugi;
+      this.cache = cache;
     }
 
     @Override
@@ -309,6 +318,18 @@ public abstract class User {
     }
 
     @Override
+    public String[] getGroupNames() {
+      if (cache != null) {
+        try {
+          return this.cache.get(ugi);
+        } catch (ExecutionException e) {
+          return new String[0];
+        }
+      }
+      return ugi.getGroupNames();
+    }
+
+    @Override
     public <T> T runAs(PrivilegedAction<T> action) {
       return ugi.doAs(action);
     }

http://git-wip-us.apache.org/repos/asf/hbase/blob/8d947730/hbase-common/src/main/java/org/apache/hadoop/hbase/security/UserProvider.java
----------------------------------------------------------------------
diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/security/UserProvider.java
b/hbase-common/src/main/java/org/apache/hadoop/hbase/security/UserProvider.java
index 82f686f..f2897b6 100644
--- a/hbase-common/src/main/java/org/apache/hadoop/hbase/security/UserProvider.java
+++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/security/UserProvider.java
@@ -18,9 +18,20 @@
 package org.apache.hadoop.hbase.security;
 
 import java.io.IOException;
+import java.util.concurrent.Callable;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
 
 import org.apache.hadoop.hbase.classification.InterfaceAudience;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.util.concurrent.ListenableFuture;
+import com.google.common.util.concurrent.ListeningExecutorService;
+import com.google.common.util.concurrent.MoreExecutors;
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.CommonConfigurationKeys;
 import org.apache.hadoop.hbase.BaseConfigurable;
 import org.apache.hadoop.security.UserGroupInformation;
 import org.apache.hadoop.util.ReflectionUtils;
@@ -33,6 +44,50 @@ import org.apache.hadoop.util.ReflectionUtils;
 public class UserProvider extends BaseConfigurable {
 
   private static final String USER_PROVIDER_CONF_KEY = "hbase.client.userprovider.class";
+  private static final ListeningExecutorService executor = MoreExecutors.listeningDecorator(
+      Executors.newScheduledThreadPool(
+          1,
+          new ThreadFactoryBuilder().setDaemon(true).setNameFormat("group-cache-%d").build()));
+
+  private LoadingCache<UserGroupInformation, String[]> groupCache = null;
+
+
+  @Override
+  public void setConf(Configuration conf) {
+    super.setConf(conf);
+    long cacheTimeout =
+        getConf().getLong(CommonConfigurationKeys.HADOOP_SECURITY_GROUPS_CACHE_SECS,
+          300) * 1000;
+
+    this.groupCache = CacheBuilder.newBuilder()
+        // This is the same timeout that hadoop uses. So we'll follow suit.
+        .refreshAfterWrite(cacheTimeout, TimeUnit.MILLISECONDS)
+        .expireAfterWrite(10 * cacheTimeout, TimeUnit.MILLISECONDS)
+            // Set concurrency level equal to the default number of handlers that
+            // the simple handler spins up.
+        .concurrencyLevel(20)
+            // create the loader
+            // This just delegates to UGI.
+        .build(new CacheLoader<UserGroupInformation, String[]>() {
+          @Override
+          public String[] load(UserGroupInformation ugi) throws Exception {
+            return ugi.getGroupNames();
+          }
+
+          // Provide the reload function that uses the executor thread.
+          public ListenableFuture<String[]> reload(final UserGroupInformation k,
+                                                   String[] oldValue) throws Exception {
+
+            return executor.submit(new Callable<String[]>() {
+              UserGroupInformation userGroupInformation = k;
+              @Override
+              public String[] call() throws Exception {
+                return userGroupInformation.getGroupNames();
+              }
+            });
+          }
+        });
+  }
 
   /**
    * Instantiate the {@link UserProvider} specified in the configuration and set the passed
@@ -95,7 +150,10 @@ public class UserProvider extends BaseConfigurable {
    * @return User
    */
   public User create(UserGroupInformation ugi) {
-    return User.create(ugi);
+    if (ugi == null) {
+      return null;
+    }
+    return new User.SecureHadoopUser(ugi, groupCache);
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/hbase/blob/8d947730/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/CallRunner.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/CallRunner.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/CallRunner.java
index 49396d7..426ba8d 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/CallRunner.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/CallRunner.java
@@ -23,7 +23,6 @@ import org.apache.hadoop.hbase.classification.InterfaceAudience;
 import org.apache.hadoop.hbase.CellScanner;
 import org.apache.hadoop.hbase.ipc.RpcServer.Call;
 import org.apache.hadoop.hbase.monitoring.MonitoredRPCHandler;
-import org.apache.hadoop.hbase.monitoring.TaskMonitor;
 import org.apache.hadoop.hbase.util.Pair;
 import org.apache.hadoop.security.UserGroupInformation;
 import org.apache.hadoop.util.StringUtils;
@@ -82,9 +81,9 @@ public class CallRunner {
       }
       this.status.setStatus("Setting up call");
       this.status.setConnection(call.connection.getHostAddress(), call.connection.getRemotePort());
-      if (RpcServer.LOG.isDebugEnabled()) {
-        UserGroupInformation remoteUser = call.connection.user;
-        RpcServer.LOG.debug(call.toShortString() + " executing as " +
+      if (RpcServer.LOG.isTraceEnabled()) {
+        UserGroupInformation remoteUser = call.connection.ugi;
+        RpcServer.LOG.trace(call.toShortString() + " executing as " +
             ((remoteUser == null) ? "NULL principal" : remoteUser.getUserName()));
       }
       Throwable errorThrowable = null;

http://git-wip-us.apache.org/repos/asf/hbase/blob/8d947730/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
index f637fe4..19ce043 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
@@ -337,11 +337,7 @@ public class RpcServer implements RpcServerInterface {
       this.isError = false;
       this.size = size;
       this.tinfo = tinfo;
-      if (connection != null && connection.user != null) {
-        this.user = userProvider.create(connection.user);
-      } else {
-        this.user = null;
-      }
+      this.user = connection.user;
       this.remoteAddress = remoteAddress;
     }
 
@@ -368,22 +364,6 @@ public class RpcServer implements RpcServerInterface {
       return this.header;
     }
 
-    @Override
-    public User getRequestUser() {
-      return user;
-    }
-
-    @Override
-    public String getRequestUserName() {
-      User user = getRequestUser();
-      return user == null? null: user.getShortName();
-    }
-
-    @Override
-    public InetAddress getRemoteAddress() {
-      return remoteAddress;
-    }
-
     /*
      * Short string representation without param info because param itself could be huge
depends on
      * the payload of a command
@@ -569,6 +549,26 @@ public class RpcServer implements RpcServerInterface {
         this.responder.doRespond(this);
       }
     }
+
+    public UserGroupInformation getRemoteUser() {
+      return connection.ugi;
+    }
+
+    @Override
+    public User getRequestUser() {
+      return user;
+    }
+
+    @Override
+    public String getRequestUserName() {
+      User user = getRequestUser();
+      return user == null? null: user.getShortName();
+    }
+
+    @Override
+    public InetAddress getRemoteAddress() {
+      return remoteAddress;
+    }
   }
 
   /** Listens on the socket. Creates jobs for the handler threads*/
@@ -1215,7 +1215,7 @@ public class RpcServer implements RpcServerInterface {
      */
     private CompressionCodec compressionCodec;
     BlockingService service;
-    protected UserGroupInformation user = null;
+
     private AuthMethod authMethod;
     private boolean saslContextEstablished;
     private boolean skipInitialSaslHandshake;
@@ -1241,6 +1241,8 @@ public class RpcServer implements RpcServerInterface {
     private boolean authenticatedWithFallback;
 
     public UserGroupInformation attemptingUser = null; // user name before auth
+    protected User user = null;
+    protected UserGroupInformation ugi = null;
 
     public Connection(SocketChannel channel, long lastContact) {
       this.channel = channel;
@@ -1422,14 +1424,14 @@ public class RpcServer implements RpcServerInterface {
         if (saslServer.isComplete()) {
           String qop = (String) saslServer.getNegotiatedProperty(Sasl.QOP);
           useWrap = qop != null && !"auth".equalsIgnoreCase(qop);
-          user = getAuthorizedUgi(saslServer.getAuthorizationID());
+          ugi = getAuthorizedUgi(saslServer.getAuthorizationID());
           if (LOG.isDebugEnabled()) {
             LOG.debug("SASL server context established. Authenticated client: "
-              + user + ". Negotiated QoP is "
+              + ugi + ". Negotiated QoP is "
               + saslServer.getNegotiatedProperty(Sasl.QOP));
           }
           metrics.authenticationSuccess();
-          AUDITLOG.info(AUTH_SUCCESSFUL_FOR + user);
+          AUDITLOG.info(AUTH_SUCCESSFUL_FOR + ugi);
           saslContextEstablished = true;
         }
       }
@@ -1630,9 +1632,9 @@ public class RpcServer implements RpcServerInterface {
       setupCellBlockCodecs(this.connectionHeader);
       UserGroupInformation protocolUser = createUser(connectionHeader);
       if (!useSasl) {
-        user = protocolUser;
-        if (user != null) {
-          user.setAuthenticationMethod(AuthMethod.SIMPLE.authenticationMethod);
+        ugi = protocolUser;
+        if (ugi != null) {
+          ugi.setAuthenticationMethod(AuthMethod.SIMPLE.authenticationMethod);
         }
         // audit logging for SASL authenticated users happens in saslReadAndProcess()
         if (authenticatedWithFallback) {
@@ -1642,26 +1644,26 @@ public class RpcServer implements RpcServerInterface {
         AUDITLOG.info(AUTH_SUCCESSFUL_FOR + user);
       } else {
         // user is authenticated
-        user.setAuthenticationMethod(authMethod.authenticationMethod);
+        ugi.setAuthenticationMethod(authMethod.authenticationMethod);
         //Now we check if this is a proxy user case. If the protocol user is
         //different from the 'user', it is a proxy user scenario. However,
         //this is not allowed if user authenticated with DIGEST.
         if ((protocolUser != null)
-            && (!protocolUser.getUserName().equals(user.getUserName()))) {
+            && (!protocolUser.getUserName().equals(ugi.getUserName()))) {
           if (authMethod == AuthMethod.DIGEST) {
             // Not allowed to doAs if token authentication is used
-            throw new AccessDeniedException("Authenticated user (" + user
+            throw new AccessDeniedException("Authenticated user (" + ugi
                 + ") doesn't match what the client claims to be ("
                 + protocolUser + ")");
           } else {
             // Effective user can be different from authenticated user
             // for simple auth or kerberos auth
             // The user is the real user. Now we create a proxy user
-            UserGroupInformation realUser = user;
-            user = UserGroupInformation.createProxyUser(protocolUser
+            UserGroupInformation realUser = ugi;
+            ugi = UserGroupInformation.createProxyUser(protocolUser
                 .getUserName(), realUser);
             // Now the user is a proxy user, set Authentication method Proxy.
-            user.setAuthenticationMethod(AuthenticationMethod.PROXY);
+            ugi.setAuthenticationMethod(AuthenticationMethod.PROXY);
           }
         }
       }
@@ -1748,8 +1750,9 @@ public class RpcServer implements RpcServerInterface {
           // Throw FatalConnectionException wrapping ACE so client does right thing and closes
           // down the connection instead of trying to read non-existent retun.
           throw new AccessDeniedException("Connection from " + this + " for service " +
-            connectionHeader.getServiceName() + " is unauthorized for user: " + user);
+            connectionHeader.getServiceName() + " is unauthorized for user: " + ugi);
         }
+        this.user = userProvider.create(this.ugi);
       }
     }
 
@@ -1855,11 +1858,11 @@ public class RpcServer implements RpcServerInterface {
         // real user for the effective user, therefore not required to
         // authorize real user. doAs is allowed only for simple or kerberos
         // authentication
-        if (user != null && user.getRealUser() != null
+        if (ugi != null && ugi.getRealUser() != null
             && (authMethod != AuthMethod.DIGEST)) {
-          ProxyUsers.authorize(user, this.getHostAddress(), conf);
+          ProxyUsers.authorize(ugi, this.getHostAddress(), conf);
         }
-        authorize(user, connectionHeader, getHostInetAddress());
+        authorize(ugi, connectionHeader, getHostInetAddress());
         if (LOG.isDebugEnabled()) {
           LOG.debug("Authorized " + TextFormat.shortDebugString(connectionHeader));
         }


Mime
View raw message