[GitHub] [gobblin] Will-Lo commented on a change in pull request #3450: [GOBBLIn-1595]Fix the dead lock during hive registration

[GitBox <gi...@apache.org>]

Will-Lo commented on a change in pull request #3450:
URL: https://github.com/apache/gobblin/pull/3450#discussion_r784247950



##########
File path: gobblin-hive-registration/src/main/java/org/apache/gobblin/hive/metastore/HiveMetaStoreBasedRegister.java
##########
@@ -472,38 +472,33 @@ private boolean createTableIfNotExists(IMetaStoreClient client, Table table, Hiv
   private void createOrAlterTable(IMetaStoreClient client, Table table, HiveSpec spec) throws TException, IOException {
     String dbName = table.getDbName();
     String tableName = table.getTableName();
-    boolean tableExistenceInCache;
-    if (this.optimizedChecks) {
-      try {
-        this.tableAndDbExistenceCache.get(dbName + ":" + tableName, new Callable<Boolean>() {
-          @Override
-          public Boolean call() throws Exception {
-            return ensureHiveTableExistenceBeforeAlternation(tableName, dbName, client, table);
-          }
-        });
-      } catch (ExecutionException ee) {
-        throw new IOException("Table existence checking throwing execution exception.", ee);
-      }
-    } else {
-      this.ensureHiveTableExistenceBeforeAlternation(tableName, dbName, client, table);
-    }
+    this.ensureHiveTableExistenceBeforeAlternation(tableName, dbName, client, table);
     alterTableIfNeeded(tableName, dbName, client, table, spec);
   }
 
-  @Override
-  public boolean existsTable(String dbName, String tableName) throws IOException {
+  public boolean existsTable(String dbName, String tableName, IMetaStoreClient client) throws IOException {
     if (this.optimizedChecks && this.tableAndDbExistenceCache.getIfPresent(dbName + ":" + tableName ) != null ) {
       return true;
     }
-    try (AutoReturnableObject<IMetaStoreClient> client = this.clientPool.getClient()) {
+    try {
+      boolean exists;
       try (Timer.Context context = this.metricContext.timer(TABLE_EXISTS).time()) {
-        return client.get().tableExists(dbName, tableName);
+        exists =  client.tableExists(dbName, tableName);
       }
+      this.tableAndDbExistenceCache.put(dbName + ":" + tableName, exists);
+      return exists;
     } catch (TException e) {
       throw new IOException(String.format("Unable to check existence of table %s in db %s", tableName, dbName), e);
     }
   }
 
+  @Override
+  public boolean existsTable(String dbName, String tableName) throws IOException {

Review comment:
       Small nit, can we name this function `tableExists` ? Reads a bit better




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org