You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by jn...@apache.org on 2015/12/11 19:25:13 UTC

[1/3] drill git commit: DRILL-4161: Make Hive Metastore client caching policy user configurable.

Repository: drill
Updated Branches:
  refs/heads/master 539cbba56 -> bb3fc1521


DRILL-4161: Make Hive Metastore client caching policy user configurable.

Remove common code.


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

Branch: refs/heads/master
Commit: bb3fc15216d9cab804fc9a6f0e5bd34597dd4394
Parents: 02ce44b
Author: Jinfeng Ni <jn...@apache.org>
Authored: Mon Dec 7 14:17:04 2015 -0800
Committer: Jinfeng Ni <jn...@apache.org>
Committed: Fri Dec 11 09:49:55 2015 -0800

----------------------------------------------------------------------
 .../store/hive/DrillHiveMetaStoreClient.java    | 45 ++++++++++++++------
 1 file changed, 33 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/bb3fc152/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/DrillHiveMetaStoreClient.java
----------------------------------------------------------------------
diff --git a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/DrillHiveMetaStoreClient.java b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/DrillHiveMetaStoreClient.java
index 2cce699..8b46a93 100644
--- a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/DrillHiveMetaStoreClient.java
+++ b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/DrillHiveMetaStoreClient.java
@@ -53,6 +53,11 @@ import java.util.concurrent.TimeUnit;
 public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient {
   private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(DrillHiveMetaStoreClient.class);
 
+  public final String HIVE_METASTORE_CACHE_TTL = "hive.metastore.cache-ttl-seconds";
+  public final String HIVE_METASTORE_CACHE_EXPIRE = "hive.metastore.cache-expire-after";
+  public final String HIVE_METASTORE_CACHE_EXPIRE_AFTER_WRITE = "write";
+  public final String HIVE_METASTORE_CACHE_EXPIRE_AFTER_ACCESS = "access";
+
   protected final Map<String, String> hiveConfigOverride;
 
   protected final LoadingCache<String, List<String>> databases;
@@ -137,22 +142,38 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient {
   private DrillHiveMetaStoreClient(final HiveConf hiveConf, final Map<String, String> hiveConfigOverride)
       throws MetaException {
     super(hiveConf);
+
+    int hmsCacheTTL = 60; // default is 60 seconds
+    boolean expireAfterWrite = true; // default is expire after write.
+
+    if (hiveConfigOverride.containsKey(HIVE_METASTORE_CACHE_TTL)) {
+      hmsCacheTTL = Integer.valueOf(hiveConfigOverride.get(HIVE_METASTORE_CACHE_TTL));
+      logger.warn("Hive metastore cache ttl is set to {} seconds.", hmsCacheTTL);
+    }
+
+    if (hiveConfigOverride.containsKey(HIVE_METASTORE_CACHE_EXPIRE)) {
+      if (hiveConfigOverride.get(HIVE_METASTORE_CACHE_EXPIRE).equalsIgnoreCase(HIVE_METASTORE_CACHE_EXPIRE_AFTER_WRITE)) {
+        expireAfterWrite = true;
+      } else if (hiveConfigOverride.get(HIVE_METASTORE_CACHE_EXPIRE).equalsIgnoreCase(HIVE_METASTORE_CACHE_EXPIRE_AFTER_ACCESS)) {
+        expireAfterWrite = false;
+      }
+      logger.warn("Hive metastore cache expire policy is set to {}", expireAfterWrite? "expireAfterWrite" : "expireAfterAccess");
+    }
+
     this.hiveConfigOverride = hiveConfigOverride;
 
-    databases = CacheBuilder //
-        .newBuilder() //
-        .expireAfterWrite(1, TimeUnit.MINUTES) //
-        .build(new DatabaseLoader());
+    final CacheBuilder cacheBuilder = CacheBuilder
+        .newBuilder();
 
-    tableNameLoader = CacheBuilder //
-        .newBuilder() //
-        .expireAfterWrite(1, TimeUnit.MINUTES) //
-        .build(new TableNameLoader());
+    if (expireAfterWrite) {
+      cacheBuilder.expireAfterWrite(hmsCacheTTL, TimeUnit.SECONDS);
+    } else {
+      cacheBuilder.expireAfterAccess(hmsCacheTTL, TimeUnit.SECONDS);
+    }
 
-    tableLoaders = CacheBuilder //
-        .newBuilder() //
-        .expireAfterWrite(1, TimeUnit.MINUTES) //
-        .build(new TableLoader());
+    databases = cacheBuilder.build(new DatabaseLoader());
+    tableNameLoader = cacheBuilder.build(new TableNameLoader());
+    tableLoaders = cacheBuilder.build(new TableLoader());
 
   }
 


[2/3] drill git commit: DRILL-4126: Enable caching for HiveMetaStore access when impersonation is enabled. Change table cache in metastore client.

Posted by jn...@apache.org.
DRILL-4126: Enable caching for HiveMetaStore access when impersonation is enabled. Change table cache in metastore client.

1) HiveSchemaFactory maintain one metastoreClientWithAuth per user.
2) Put the hive metastore object caches in DrillHiveMetaStoreClient.
3) Use LoadingCache to store the set of clients for impersonation.
4) Use flat level cache for tables in DrillHiveMetastoreClient.


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/02ce44bc
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/02ce44bc
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/02ce44bc

Branch: refs/heads/master
Commit: 02ce44bcab37bce0176e472b194d16acb6d167b7
Parents: b8b18eb
Author: Jinfeng Ni <jn...@apache.org>
Authored: Tue Nov 24 13:38:12 2015 -0800
Committer: Jinfeng Ni <jn...@apache.org>
Committed: Fri Dec 11 09:49:55 2015 -0800

----------------------------------------------------------------------
 .../store/hive/DrillHiveMetaStoreClient.java    | 201 ++++++++++++-------
 .../store/hive/schema/HiveSchemaFactory.java    |  41 +++-
 2 files changed, 160 insertions(+), 82 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/02ce44bc/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/DrillHiveMetaStoreClient.java
----------------------------------------------------------------------
diff --git a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/DrillHiveMetaStoreClient.java b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/DrillHiveMetaStoreClient.java
index 3667c8f..2cce699 100644
--- a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/DrillHiveMetaStoreClient.java
+++ b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/DrillHiveMetaStoreClient.java
@@ -42,6 +42,7 @@ import java.security.PrivilegedExceptionAction;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 
@@ -54,6 +55,11 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient {
 
   protected final Map<String, String> hiveConfigOverride;
 
+  protected final LoadingCache<String, List<String>> databases;
+  protected final LoadingCache<String, List<String>> tableNameLoader;
+  protected final LoadingCache<TableName, HiveReadEntry> tableLoaders;
+
+
   /**
    * Create a DrillHiveMetaStoreClient for cases where:
    *   1. Drill impersonation is enabled and
@@ -71,8 +77,7 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient {
    * @throws MetaException
    */
   public static DrillHiveMetaStoreClient createClientWithAuthz(final DrillHiveMetaStoreClient processUserMetaStoreClient,
-      final HiveConf hiveConf, final Map<String, String> hiveConfigOverride, final String userName,
-      final boolean ignoreAuthzErrors) throws MetaException {
+      final HiveConf hiveConf, final Map<String, String> hiveConfigOverride, final String userName) throws MetaException {
     try {
       boolean delegationTokenGenerated = false;
 
@@ -89,7 +94,7 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient {
           // delegation tokens).
           String delegationToken = processUserMetaStoreClient.getDelegationToken(userName, userName);
           try {
-            ShimLoader.getHadoopShims().setTokenStr(ugiForRpc, delegationToken, HiveClientWithAuthz.DRILL2HMS_TOKEN);
+            ShimLoader.getHadoopShims().setTokenStr(ugiForRpc, delegationToken, HiveClientWithAuthzWithCaching.DRILL2HMS_TOKEN);
           } catch (IOException e) {
             throw new DrillRuntimeException("Couldn't setup delegation token in the UGI for Hive MetaStoreClient", e);
           }
@@ -100,7 +105,7 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient {
       final HiveConf hiveConfForClient;
       if (delegationTokenGenerated) {
         hiveConfForClient = new HiveConf(hiveConf);
-        hiveConfForClient.set("hive.metastore.token.signature", HiveClientWithAuthz.DRILL2HMS_TOKEN);
+        hiveConfForClient.set("hive.metastore.token.signature", HiveClientWithAuthzWithCaching.DRILL2HMS_TOKEN);
       } else {
         hiveConfForClient = hiveConf;
       }
@@ -108,7 +113,7 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient {
       return ugiForRpc.doAs(new PrivilegedExceptionAction<DrillHiveMetaStoreClient>() {
         @Override
         public DrillHiveMetaStoreClient run() throws Exception {
-          return new HiveClientWithAuthz(hiveConfForClient, hiveConfigOverride, ugiForRpc, userName, ignoreAuthzErrors);
+          return new HiveClientWithAuthzWithCaching(hiveConfForClient, hiveConfigOverride, ugiForRpc, userName);
         }
       });
     } catch (final Exception e) {
@@ -133,15 +138,30 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient {
       throws MetaException {
     super(hiveConf);
     this.hiveConfigOverride = hiveConfigOverride;
-  }
 
+    databases = CacheBuilder //
+        .newBuilder() //
+        .expireAfterWrite(1, TimeUnit.MINUTES) //
+        .build(new DatabaseLoader());
+
+    tableNameLoader = CacheBuilder //
+        .newBuilder() //
+        .expireAfterWrite(1, TimeUnit.MINUTES) //
+        .build(new TableNameLoader());
+
+    tableLoaders = CacheBuilder //
+        .newBuilder() //
+        .expireAfterWrite(1, TimeUnit.MINUTES) //
+        .build(new TableLoader());
+
+  }
 
   /**
    * Higher level API that returns the databases in Hive.
    * @return
    * @throws TException
    */
-  public abstract List<String> getDatabases() throws TException;
+  public abstract List<String> getDatabases(boolean ignoreAuthzErrors) throws TException;
 
   /**
    * Higher level API that returns the tables in given database.
@@ -149,7 +169,7 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient {
    * @return
    * @throws TException
    */
-  public abstract List<String> getTableNames(final String dbName) throws TException;
+  public abstract List<String> getTableNames(final String dbName, boolean ignoreAuthzErrors) throws TException;
 
   /**
    * Higher level API that returns the {@link HiveReadEntry} for given database and table.
@@ -158,7 +178,7 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient {
    * @return
    * @throws TException
    */
-  public abstract HiveReadEntry getHiveReadEntry(final String dbName, final String tableName) throws TException;
+  public abstract HiveReadEntry getHiveReadEntry(final String dbName, final String tableName, boolean ignoreAuthzErrors) throws TException;
 
   /** Helper method which gets database. Retries once if the first call to fetch the metadata fails */
   protected static List<String> getDatabasesHelper(final IMetaStoreClient mClient) throws TException {
@@ -222,19 +242,17 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient {
    * HiveMetaStoreClient to create and maintain (reconnection cases) connection to Hive metastore with given user
    * credentials and check authorization privileges if set.
    */
-  private static class HiveClientWithAuthz extends DrillHiveMetaStoreClient {
+  private static class HiveClientWithAuthzWithCaching extends DrillHiveMetaStoreClient {
     public static final String DRILL2HMS_TOKEN = "DrillDelegationTokenForHiveMetaStoreServer";
 
     private final UserGroupInformation ugiForRpc;
-    private final boolean ignoreAuthzErrors;
     private HiveAuthorizationHelper authorizer;
 
-    private HiveClientWithAuthz(final HiveConf hiveConf, final Map<String, String> hiveConfigOverride,
-        final UserGroupInformation ugiForRpc, final String userName, final boolean ignoreAuthzErrors)
+    private HiveClientWithAuthzWithCaching(final HiveConf hiveConf, final Map<String, String> hiveConfigOverride,
+        final UserGroupInformation ugiForRpc, final String userName)
         throws TException {
       super(hiveConf, hiveConfigOverride);
       this.ugiForRpc = ugiForRpc;
-      this.ignoreAuthzErrors = ignoreAuthzErrors;
       this.authorizer = new HiveAuthorizationHelper(this, hiveConf, userName);
     }
 
@@ -257,7 +275,8 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient {
       super.reconnect();
     }
 
-    public List<String> getDatabases() throws TException {
+    @Override
+    public List<String> getDatabases(boolean ignoreAuthzErrors) throws TException {
       try {
         authorizer.authorizeShowDatabases();
       } catch (final HiveAccessControlException e) {
@@ -266,10 +285,16 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient {
         }
         throw UserException.permissionError(e).build(logger);
       }
-      return getDatabasesHelper(this);
+
+      try {
+        return databases.get("databases");
+      } catch (final ExecutionException e) {
+        throw new TException(e);
+      }
     }
 
-    public List<String> getTableNames(final String dbName) throws TException {
+    @Override
+    public List<String> getTableNames(final String dbName, boolean ignoreAuthzErrors) throws TException {
       try {
         authorizer.authorizeShowTables(dbName);
       } catch (final HiveAccessControlException e) {
@@ -278,10 +303,16 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient {
         }
         throw UserException.permissionError(e).build(logger);
       }
-      return getTableNamesHelper(this, dbName);
+
+      try {
+        return tableNameLoader.get(dbName);
+      } catch (final ExecutionException e) {
+        throw new TException(e);
+      }
     }
 
-    public HiveReadEntry getHiveReadEntry(final String dbName, final String tableName) throws TException {
+    @Override
+    public HiveReadEntry getHiveReadEntry(final String dbName, final String tableName, boolean ignoreAuthzErrors) throws TException {
       try {
         authorizer.authorizeReadTable(dbName, tableName);
       } catch (final HiveAccessControlException e) {
@@ -289,41 +320,27 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient {
           throw UserException.permissionError(e).build(logger);
         }
       }
-      return getHiveReadEntryHelper(this, dbName, tableName, hiveConfigOverride);
+
+      try {
+        return tableLoaders.get(TableName.table(dbName,tableName));
+      } catch (final ExecutionException e) {
+        throw new TException(e);
+      }
     }
+
   }
 
   /**
    * HiveMetaStoreClient that provides a shared MetaStoreClient implementation with caching.
    */
   private static class NonCloseableHiveClientWithCaching extends DrillHiveMetaStoreClient {
-    private final LoadingCache<String, List<String>> databases;
-    private final LoadingCache<String, List<String>> tableNameLoader;
-    private final LoadingCache<String, LoadingCache<String, HiveReadEntry>> tableLoaders;
-
     private NonCloseableHiveClientWithCaching(final HiveConf hiveConf,
         final Map<String, String> hiveConfigOverride) throws MetaException {
       super(hiveConf, hiveConfigOverride);
-
-      databases = CacheBuilder //
-          .newBuilder() //
-          .expireAfterWrite(1, TimeUnit.MINUTES) //
-          .build(new DatabaseLoader());
-
-      tableNameLoader = CacheBuilder //
-          .newBuilder() //
-          .expireAfterWrite(1, TimeUnit.MINUTES) //
-          .build(new TableNameLoader());
-
-      tableLoaders = CacheBuilder //
-          .newBuilder() //
-          .expireAfterAccess(4, TimeUnit.HOURS) //
-          .maximumSize(20) //
-          .build(new TableLoaderLoader());
     }
 
     @Override
-    public List<String> getDatabases() throws TException {
+    public List<String> getDatabases(boolean ignoreAuthzErrors) throws TException {
       try {
         return databases.get("databases");
       } catch (final ExecutionException e) {
@@ -332,7 +349,7 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient {
     }
 
     @Override
-    public List<String> getTableNames(final String dbName) throws TException {
+    public List<String> getTableNames(final String dbName, boolean ignoreAuthzErrors) throws TException {
       try {
         return tableNameLoader.get(dbName);
       } catch (final ExecutionException e) {
@@ -341,9 +358,9 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient {
     }
 
     @Override
-    public HiveReadEntry getHiveReadEntry(final String dbName, final String tableName) throws TException {
+    public HiveReadEntry getHiveReadEntry(final String dbName, final String tableName, boolean ignoreAuthzErrors) throws TException {
       try {
-        return tableLoaders.get(dbName).get(tableName);
+        return tableLoaders.get(TableName.table(dbName,tableName));
       } catch (final ExecutionException e) {
         throw new TException(e);
       }
@@ -361,50 +378,82 @@ public abstract class DrillHiveMetaStoreClient extends HiveMetaStoreClient {
       // No-op.
     }
 
-    private class DatabaseLoader extends CacheLoader<String, List<String>> {
-      @Override
-      public List<String> load(String key) throws Exception {
-        if (!"databases".equals(key)) {
-          throw new UnsupportedOperationException();
-        }
-        synchronized (NonCloseableHiveClientWithCaching.this) {
-          return getDatabasesHelper(NonCloseableHiveClientWithCaching.this);
-        }
+  }
+
+  private class DatabaseLoader extends CacheLoader<String, List<String>> {
+    @Override
+    public List<String> load(String key) throws Exception {
+      if (!"databases".equals(key)) {
+        throw new UnsupportedOperationException();
+      }
+      synchronized (DrillHiveMetaStoreClient.this) {
+        return getDatabasesHelper(DrillHiveMetaStoreClient.this);
       }
     }
+  }
 
-    private class TableNameLoader extends CacheLoader<String, List<String>> {
-      @Override
-      public List<String> load(String dbName) throws Exception {
-        synchronized (NonCloseableHiveClientWithCaching.this) {
-          return getTableNamesHelper(NonCloseableHiveClientWithCaching.this, dbName);
-        }
+  private class TableNameLoader extends CacheLoader<String, List<String>> {
+    @Override
+    public List<String> load(String dbName) throws Exception {
+      synchronized (DrillHiveMetaStoreClient.this) {
+        return getTableNamesHelper(DrillHiveMetaStoreClient.this, dbName);
       }
     }
+  }
 
-    private class TableLoaderLoader extends CacheLoader<String, LoadingCache<String, HiveReadEntry>> {
-      @Override
-      public LoadingCache<String, HiveReadEntry> load(String key) throws Exception {
-        return CacheBuilder
-            .newBuilder()
-            .expireAfterWrite(1, TimeUnit.MINUTES)
-            .build(new TableLoader(key));
+  private class TableLoader extends CacheLoader<TableName, HiveReadEntry> {
+    @Override
+    public HiveReadEntry load(TableName key) throws Exception {
+      synchronized (DrillHiveMetaStoreClient.this) {
+        return getHiveReadEntryHelper(DrillHiveMetaStoreClient.this, key.getDatabaseName(), key.getTableName(), hiveConfigOverride);
       }
     }
+  }
 
-    private class TableLoader extends CacheLoader<String, HiveReadEntry> {
-      private final String dbName;
+  static class TableName {
+    private final String databaseName;
+    private final String tableName;
 
-      public TableLoader(final String dbName) {
-        this.dbName = dbName;
-      }
+    private TableName(String databaseName, String tableName) {
+      this.databaseName = databaseName;
+      this.tableName = tableName;
+    }
 
-      @Override
-      public HiveReadEntry load(String key) throws Exception {
-        synchronized (NonCloseableHiveClientWithCaching.this) {
-          return getHiveReadEntryHelper(NonCloseableHiveClientWithCaching.this, dbName, key, hiveConfigOverride);
-        }
+    public static TableName table(String databaseName, String tableName) {
+      return new TableName(databaseName, tableName);
+    }
+
+    public String getDatabaseName() {
+      return databaseName;
+    }
+
+    public String getTableName() {
+      return tableName;
+    }
+
+    @Override
+    public String toString() {
+      return String.format("databaseName:%s, tableName:%s", databaseName, tableName).toString();
+    }
+
+    @Override
+    public boolean equals(Object o) {
+      if (this == o) {
+        return true;
+      }
+      if (o == null || getClass() != o.getClass()) {
+        return false;
       }
+
+      TableName other = (TableName) o;
+      return Objects.equals(databaseName, other.databaseName) &&
+          Objects.equals(tableName, other.tableName);
+    }
+
+    @Override
+    public int hashCode() {
+      return Objects.hash(databaseName, tableName);
     }
   }
+
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/02ce44bc/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveSchemaFactory.java
----------------------------------------------------------------------
diff --git a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveSchemaFactory.java b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveSchemaFactory.java
index 16aca54..05ab3a7 100644
--- a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveSchemaFactory.java
+++ b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveSchemaFactory.java
@@ -21,7 +21,15 @@ import java.io.IOException;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+
+import com.google.common.base.Stopwatch;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.cache.RemovalListener;
+import com.google.common.cache.RemovalNotification;
 import org.apache.calcite.schema.SchemaPlus;
 
 import org.apache.drill.common.exceptions.ExecutionSetupException;
@@ -48,6 +56,9 @@ public class HiveSchemaFactory implements SchemaFactory {
 
   // MetaStoreClient created using process user credentials
   private final DrillHiveMetaStoreClient processUserMetastoreClient;
+  // MetasStoreClient created using SchemaConfig credentials
+  private final LoadingCache<String, DrillHiveMetaStoreClient> metaStoreClientLoadingCache;
+
   private final HiveStoragePlugin plugin;
   private final Map<String, String> hiveConfigOverride;
   private final String schemaName;
@@ -55,7 +66,7 @@ public class HiveSchemaFactory implements SchemaFactory {
   private final boolean isDrillImpersonationEnabled;
   private final boolean isHS2DoAsSet;
 
-  public HiveSchemaFactory(HiveStoragePlugin plugin, String name, Map<String, String> hiveConfigOverride) throws ExecutionSetupException {
+  public HiveSchemaFactory(final HiveStoragePlugin plugin, final String name, final Map<String, String> hiveConfigOverride) throws ExecutionSetupException {
     this.schemaName = name;
     this.plugin = plugin;
 
@@ -79,6 +90,25 @@ public class HiveSchemaFactory implements SchemaFactory {
     } catch (MetaException e) {
       throw new ExecutionSetupException("Failure setting up Hive metastore client.", e);
     }
+
+    metaStoreClientLoadingCache = CacheBuilder
+        .newBuilder()
+        .expireAfterAccess(10, TimeUnit.MINUTES)
+        .maximumSize(5) // Up to 5 clients for impersonation-enabled.
+        .removalListener(new RemovalListener<String, DrillHiveMetaStoreClient>() {
+          @Override
+          public void onRemoval(RemovalNotification<String, DrillHiveMetaStoreClient> notification) {
+            DrillHiveMetaStoreClient client = notification.getValue();
+            client.close();
+          }
+        })
+        .build(new CacheLoader<String, DrillHiveMetaStoreClient>() {
+          @Override
+          public DrillHiveMetaStoreClient load(String userName) throws Exception {
+            return DrillHiveMetaStoreClient.createClientWithAuthz(processUserMetastoreClient, hiveConf,
+                HiveSchemaFactory.this.hiveConfigOverride, userName);
+          }
+        });
   }
 
   /**
@@ -94,9 +124,8 @@ public class HiveSchemaFactory implements SchemaFactory {
     DrillHiveMetaStoreClient mClientForSchemaTree = processUserMetastoreClient;
     if (isDrillImpersonationEnabled) {
       try {
-        mClientForSchemaTree = DrillHiveMetaStoreClient.createClientWithAuthz(processUserMetastoreClient, hiveConf,
-            hiveConfigOverride, schemaConfig.getUserName(), schemaConfig.getIgnoreAuthErrors());
-      } catch (final TException e) {
+        mClientForSchemaTree = metaStoreClientLoadingCache.get(schemaConfig.getUserName());
+      } catch (final ExecutionException e) {
         throw new IOException("Failure setting up Hive metastore client.", e);
       }
     }
@@ -202,7 +231,7 @@ public class HiveSchemaFactory implements SchemaFactory {
         dbName = "default";
       }
       try{
-        return mClient.getHiveReadEntry(dbName, t);
+        return mClient.getHiveReadEntry(dbName, t, schemaConfig.getIgnoreAuthErrors());
       }catch(final TException e) {
         logger.warn("Exception occurred while trying to read table. {}.{}", dbName, t, e.getCause());
         return null;


[3/3] drill git commit: DRILL-4127: Reduce Hive metastore client API call in HiveSchema.

Posted by jn...@apache.org.
DRILL-4127: Reduce Hive metastore client API call in HiveSchema.

1) Use lazy loading of tableNames in HiveSchema, in stead of pre-loading all table names under each HiveSchema.
2) Do not call get_all_databases for subSchema to check existence if the name comes from getSubSchemaNames() directly.

review comments.


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

Branch: refs/heads/master
Commit: b8b18ebf4b9dbf2027d1f5e9666504dbe11901cb
Parents: 539cbba
Author: Jinfeng Ni <jn...@apache.org>
Authored: Wed Nov 18 20:18:51 2015 -0800
Committer: Jinfeng Ni <jn...@apache.org>
Committed: Fri Dec 11 09:49:55 2015 -0800

----------------------------------------------------------------------
 .../store/hive/schema/HiveDatabaseSchema.java   | 24 ++++++++++++++++----
 .../store/hive/schema/HiveSchemaFactory.java    | 22 ++++++++----------
 2 files changed, 28 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/b8b18ebf/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveDatabaseSchema.java
----------------------------------------------------------------------
diff --git a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveDatabaseSchema.java b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveDatabaseSchema.java
index 48c034e..6f43639 100644
--- a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveDatabaseSchema.java
+++ b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveDatabaseSchema.java
@@ -17,30 +17,36 @@
  */
 package org.apache.drill.exec.store.hive.schema;
 
-import java.util.List;
 import java.util.Set;
 
 import org.apache.calcite.schema.Table;
 
 import org.apache.drill.exec.store.AbstractSchema;
+import org.apache.drill.exec.store.SchemaConfig;
+import org.apache.drill.exec.store.hive.DrillHiveMetaStoreClient;
 import org.apache.drill.exec.store.hive.HiveStoragePluginConfig;
 import org.apache.drill.exec.store.hive.schema.HiveSchemaFactory.HiveSchema;
 
 import com.google.common.collect.Sets;
+import org.apache.thrift.TException;
 
 public class HiveDatabaseSchema extends AbstractSchema{
   static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(HiveDatabaseSchema.class);
 
   private final HiveSchema hiveSchema;
-  private final Set<String> tables;
+  private Set<String> tables;
+  private final DrillHiveMetaStoreClient mClient;
+  private final SchemaConfig schemaConfig;
 
   public HiveDatabaseSchema( //
-      List<String> tableList, //
       HiveSchema hiveSchema, //
-      String name) {
+      String name,
+      DrillHiveMetaStoreClient mClient,
+      SchemaConfig schemaConfig) {
     super(hiveSchema.getSchemaPath(), name);
     this.hiveSchema = hiveSchema;
-    this.tables = Sets.newHashSet(tableList);
+    this.mClient = mClient;
+    this.schemaConfig = schemaConfig;
   }
 
   @Override
@@ -50,6 +56,14 @@ public class HiveDatabaseSchema extends AbstractSchema{
 
   @Override
   public Set<String> getTableNames() {
+    if (tables == null) {
+      try {
+        tables = Sets.newHashSet(mClient.getTableNames(this.name, schemaConfig.getIgnoreAuthErrors()));
+      } catch (final TException e) {
+        logger.warn("Failure while attempting to access HiveDatabase '{}'.", this.name, e.getCause());
+        tables = Sets.newHashSet(); // empty set.
+      }
+    }
     return tables;
   }
 

http://git-wip-us.apache.org/repos/asf/drill/blob/b8b18ebf/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveSchemaFactory.java
----------------------------------------------------------------------
diff --git a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveSchemaFactory.java b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveSchemaFactory.java
index 73e7bf7..16aca54 100644
--- a/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveSchemaFactory.java
+++ b/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveSchemaFactory.java
@@ -120,15 +120,13 @@ public class HiveSchemaFactory implements SchemaFactory {
 
     @Override
     public AbstractSchema getSubSchema(String name) {
-      List<String> tables;
       try {
-        List<String> dbs = mClient.getDatabases();
+        List<String> dbs = mClient.getDatabases(schemaConfig.getIgnoreAuthErrors());
         if (!dbs.contains(name)) {
           logger.debug("Database '{}' doesn't exists in Hive storage '{}'", name, schemaName);
           return null;
         }
-        tables = mClient.getTableNames(name);
-        HiveDatabaseSchema schema = new HiveDatabaseSchema(tables, this, name);
+        HiveDatabaseSchema schema = getSubSchemaKnownExists(name);
         if (name.equals("default")) {
           this.defaultSchema = schema;
         }
@@ -137,12 +135,17 @@ public class HiveSchemaFactory implements SchemaFactory {
         logger.warn("Failure while attempting to access HiveDatabase '{}'.", name, e.getCause());
         return null;
       }
+    }
 
+    /** Help method to get subschema when we know it exists (already checks the existence) */
+    private HiveDatabaseSchema getSubSchemaKnownExists(String name) {
+      HiveDatabaseSchema schema = new HiveDatabaseSchema(this, name, mClient, schemaConfig);
+      return schema;
     }
 
     void setHolder(SchemaPlus plusOfThis) {
       for (String s : getSubSchemaNames()) {
-        plusOfThis.add(s, getSubSchema(s));
+        plusOfThis.add(s, getSubSchemaKnownExists(s));
       }
     }
 
@@ -154,7 +157,7 @@ public class HiveSchemaFactory implements SchemaFactory {
     @Override
     public Set<String> getSubSchemaNames() {
       try {
-        List<String> dbs = mClient.getDatabases();
+        List<String> dbs = mClient.getDatabases(schemaConfig.getIgnoreAuthErrors());
         return Sets.newHashSet(dbs);
       } catch (final TException e) {
         logger.warn("Failure while getting Hive database list.", e);
@@ -215,13 +218,6 @@ public class HiveSchemaFactory implements SchemaFactory {
     public String getTypeName() {
       return HiveStoragePluginConfig.NAME;
     }
-
-    @Override
-    public void close() throws Exception {
-      if (mClient != null) {
-        mClient.close();
-      }
-    }
   }
 
 }