You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hive.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2020/07/02 13:42:00 UTC

[jira] [Work logged] (HIVE-22015) [CachedStore] Cache table constraints in CachedStore

     [ https://issues.apache.org/jira/browse/HIVE-22015?focusedWorklogId=453958&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-453958 ]

ASF GitHub Bot logged work on HIVE-22015:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 02/Jul/20 13:41
            Start Date: 02/Jul/20 13:41
    Worklog Time Spent: 10m 
      Work Description: sankarh commented on a change in pull request #1109:
URL: https://github.com/apache/hive/pull/1109#discussion_r447415363



##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
##########
@@ -2497,26 +2610,87 @@ long getPartsFound() {
 
   @Override public List<SQLPrimaryKey> getPrimaryKeys(String catName, String dbName, String tblName)
       throws MetaException {
-    // TODO constraintCache
-    return rawStore.getPrimaryKeys(catName, dbName, tblName);
+    catName = normalizeIdentifier(catName);
+    dbName = StringUtils.normalizeIdentifier(dbName);
+    tblName = StringUtils.normalizeIdentifier(tblName);
+    if (!shouldCacheTable(catName, dbName, tblName) || (canUseEvents && rawStore.isActiveTransaction())) {
+      return rawStore.getPrimaryKeys(catName, dbName, tblName);
+    }
+
+    Table tbl = sharedCache.getTableFromCache(catName, dbName, tblName);
+    if (tbl == null) {
+      // The table containing the primary keys is not yet loaded in cache
+      return rawStore.getPrimaryKeys(catName, dbName, tblName);
+    }
+    List<SQLPrimaryKey> keys = sharedCache.listCachedPrimaryKeys(catName, dbName, tblName);
+
+    return keys;
   }
 
   @Override public List<SQLForeignKey> getForeignKeys(String catName, String parentDbName, String parentTblName,
       String foreignDbName, String foreignTblName) throws MetaException {
-    // TODO constraintCache
-    return rawStore.getForeignKeys(catName, parentDbName, parentTblName, foreignDbName, foreignTblName);
+     // Get correct ForeignDBName and TableName
+    if (foreignDbName == null || foreignTblName == null) {
+      return rawStore.getForeignKeys(catName, parentDbName, parentTblName, foreignDbName, foreignTblName);

Review comment:
       This flow is a candidate for improvement as it tries to fetch all foreignkeys of give parent table and vice-versa which is frequent operations. Pls create a follow-up JIRA to use CachedStore for this case too.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
##########
@@ -2497,26 +2610,87 @@ long getPartsFound() {
 
   @Override public List<SQLPrimaryKey> getPrimaryKeys(String catName, String dbName, String tblName)
       throws MetaException {
-    // TODO constraintCache
-    return rawStore.getPrimaryKeys(catName, dbName, tblName);
+    catName = normalizeIdentifier(catName);
+    dbName = StringUtils.normalizeIdentifier(dbName);
+    tblName = StringUtils.normalizeIdentifier(tblName);
+    if (!shouldCacheTable(catName, dbName, tblName) || (canUseEvents && rawStore.isActiveTransaction())) {
+      return rawStore.getPrimaryKeys(catName, dbName, tblName);
+    }
+
+    Table tbl = sharedCache.getTableFromCache(catName, dbName, tblName);
+    if (tbl == null) {
+      // The table containing the primary keys is not yet loaded in cache
+      return rawStore.getPrimaryKeys(catName, dbName, tblName);
+    }
+    List<SQLPrimaryKey> keys = sharedCache.listCachedPrimaryKeys(catName, dbName, tblName);
+
+    return keys;
   }
 
   @Override public List<SQLForeignKey> getForeignKeys(String catName, String parentDbName, String parentTblName,
       String foreignDbName, String foreignTblName) throws MetaException {
-    // TODO constraintCache
-    return rawStore.getForeignKeys(catName, parentDbName, parentTblName, foreignDbName, foreignTblName);
+     // Get correct ForeignDBName and TableName
+    if (foreignDbName == null || foreignTblName == null) {

Review comment:
       We should take the same path if parentDbName or parentTblName is null.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
##########
@@ -867,6 +909,77 @@ private void updateTableColStats(RawStore rawStore, String catName, String dbNam
       }
     }
 
+    private void updateTableForeignKeys(RawStore rawStore, String catName, String dbName, String tblName) {
+      LOG.debug("CachedStore: updating cached foreign keys objects for catalog: {}, database: {}, table: {}", catName,
+              dbName, tblName);
+      try {
+        Deadline.startTimer("getForeignKeys");
+        List<SQLForeignKey> fks = rawStore.getForeignKeys(catName, null, null, dbName, tblName);
+        Deadline.stopTimer();
+        sharedCache.refreshForeignKeysInCache(StringUtils.normalizeIdentifier(catName),
+                StringUtils.normalizeIdentifier(dbName), StringUtils.normalizeIdentifier(tblName), fks);
+        LOG.debug("CachedStore: updated cached foreign keys objects for catalog: {}, database: {}, table: {}", catName,
+                dbName, tblName);
+      } catch (MetaException e) {
+        LOG.info("Updating CachedStore: unable to read foreign keys of catalog: " + catName + ", database: "

Review comment:
       Method is to "update" but the log msg says "read"

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
##########
@@ -1788,6 +2082,58 @@ public void addPartitionToCache(String catName, String dbName, String tblName, P
     }
   }
 
+  public void addPrimaryKeysToCache(String catName, String dbName, String tblName, List<SQLPrimaryKey> keys) {
+    try {
+      cacheLock.readLock().lock();

Review comment:
       We are writing in to cache. Check if we need read or write lock here? Looks like we take write lock on the table. Pls confirm if this is fine.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
##########
@@ -867,6 +909,77 @@ private void updateTableColStats(RawStore rawStore, String catName, String dbNam
       }
     }
 
+    private void updateTableForeignKeys(RawStore rawStore, String catName, String dbName, String tblName) {
+      LOG.debug("CachedStore: updating cached foreign keys objects for catalog: {}, database: {}, table: {}", catName,
+              dbName, tblName);
+      try {
+        Deadline.startTimer("getForeignKeys");
+        List<SQLForeignKey> fks = rawStore.getForeignKeys(catName, null, null, dbName, tblName);
+        Deadline.stopTimer();
+        sharedCache.refreshForeignKeysInCache(StringUtils.normalizeIdentifier(catName),
+                StringUtils.normalizeIdentifier(dbName), StringUtils.normalizeIdentifier(tblName), fks);
+        LOG.debug("CachedStore: updated cached foreign keys objects for catalog: {}, database: {}, table: {}", catName,
+                dbName, tblName);
+      } catch (MetaException e) {

Review comment:
       Shall limit the try-catch block only for rawStore calls.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
##########
@@ -1434,6 +1699,35 @@ public boolean populateTableInCache(Table table, ColumnStatistics tableColStats,
     tblWrapper.isTableColStatsCacheDirty.set(false);
     tblWrapper.isPartitionColStatsCacheDirty.set(false);
     tblWrapper.isAggrPartitionColStatsCacheDirty.set(false);
+
+    if (cacheObjects.getPrimaryKeys() != null) {
+      if(!tblWrapper.cachePrimaryKeys(cacheObjects.getPrimaryKeys(), true)) {
+        return false;
+      }
+    }
+    tblWrapper.memberCacheDirty[MemberName.PRIMARY_KEY_CACHE.ordinal()].set(false);

Review comment:
       TableWrapper can have setMemberCacheDirty(MemberName, boolean) method to make it clean.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
##########
@@ -266,6 +272,13 @@ public int getObjectSize(Class<?> clazz, Object obj) {
     private int partitionColStatsCacheSize;

Review comment:
       We shall remove these variables for stats and partition too as we already have memberObjectsSize and memberCacheDirty.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
##########
@@ -514,6 +655,130 @@ public boolean containsPartition(List<String> partVals) {
       return containsPart;
     }
 
+    public void removeConstraint(String name) {
+      try {
+        tableLock.writeLock().lock();
+        Object constraint = null;
+        MemberName mn = null;
+        Class constraintClass = null;
+        if (this.primaryKeyCache.containsKey(name)) {
+          constraint = this.primaryKeyCache.remove(name);
+          mn = MemberName.PRIMARY_KEY_CACHE;
+          this.memberCacheDirty[mn.ordinal()].set(true);

Review comment:
       memberCacheDirty.set is common code and can be moved outside if-else blocks.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
##########
@@ -543,10 +556,24 @@ static void prewarm(RawStore rawStore) {
                 tableColStats = rawStore.getTableColumnStatistics(catName, dbName, tblName, colNames, CacheUtils.HIVE_ENGINE);
                 Deadline.stopTimer();
               }
+              Deadline.startTimer("getPrimaryKeys");
+              rawStore.getPrimaryKeys(catName, dbName, tblName);
+              Deadline.stopTimer();
+              Deadline.startTimer("getForeignKeys");
+              rawStore.getForeignKeys(catName, null, null, dbName, tblName);
+              Deadline.stopTimer();
+              Deadline.startTimer("getUniqueConstraints");
+              rawStore.getUniqueConstraints(catName, dbName, tblName);
+              Deadline.stopTimer();
+              Deadline.startTimer("getNotNullConstraints");
+              rawStore.getNotNullConstraints(catName, dbName, tblName);
+              Deadline.stopTimer();
+
               // If the table could not cached due to memory limit, stop prewarm
               boolean isSuccess = sharedCache
                   .populateTableInCache(table, tableColStats, partitions, partitionColStats, aggrStatsAllPartitions,

Review comment:
       I think, we can make this refactoring also here itself. It needs change only in this method and so less risky. Having another ticket for this is time consuming.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
##########
@@ -470,6 +510,107 @@ boolean cachePartitions(Iterable<Partition> parts, SharedCache sharedCache, bool
       }
     }
 
+    boolean cachePrimaryKeys(List<SQLPrimaryKey> primaryKeys, boolean fromPrewarm) {
+      return cacheConstraints(primaryKeys, fromPrewarm, MemberName.PRIMARY_KEY_CACHE);
+    }
+
+    boolean cacheForeignKeys(List<SQLForeignKey> foreignKeys, boolean fromPrewarm) {
+      return cacheConstraints(foreignKeys, fromPrewarm, MemberName.FOREIGN_KEY_CACHE);
+    }
+
+    boolean cacheUniqueConstraints(List<SQLUniqueConstraint> uniqueConstraints, boolean fromPrewarm) {
+      return cacheConstraints(uniqueConstraints, fromPrewarm, MemberName.UNIQUE_CONSTRAINT_CACHE);
+    }
+
+    boolean cacheNotNulConstraints(List<SQLNotNullConstraint> notNullConstraints, boolean fromPrewarm) {
+      return cacheConstraints(notNullConstraints, fromPrewarm, MemberName.NOTNULL_CONSTRAINT_CACHE);
+    }
+
+    // Common method to cache constraints
+    private boolean cacheConstraints(List constraintsList,
+                             boolean fromPrewarm,
+                             MemberName mn) {
+      if (constraintsList == null || constraintsList.isEmpty()) {
+        return true;
+      }
+      try {
+        tableLock.writeLock().lock();
+        int size = 0;
+        for(int i=0; i<constraintsList.size(); i++) {

Review comment:
       Shall do the following to avoid using variable i and get(i). 
   constraintsList.stream().forEach(constraint -> {
     switch (mn) {
       case PRIMARY_KEY_CACHE:
           SQLPrimaryKey pk = (SQLPrimaryKey) constraint;
           this.primaryKeyCache.put(pk.getPk_name(), pk);
           size += getObjectSize(SQLPrimaryKey.class, constraint);
           break;
       ...
     }
   });
   

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
##########
@@ -2497,26 +2599,82 @@ long getPartsFound() {
 
   @Override public List<SQLPrimaryKey> getPrimaryKeys(String catName, String dbName, String tblName)
       throws MetaException {
-    // TODO constraintCache
-    return rawStore.getPrimaryKeys(catName, dbName, tblName);
+    catName = normalizeIdentifier(catName);
+    dbName = StringUtils.normalizeIdentifier(dbName);
+    tblName = StringUtils.normalizeIdentifier(tblName);
+    if (!shouldCacheTable(catName, dbName, tblName) || (canUseEvents && rawStore.isActiveTransaction())) {
+      return rawStore.getPrimaryKeys(catName, dbName, tblName);
+    }
+
+    Table tbl = sharedCache.getTableFromCache(catName, dbName, tblName);
+    if (tbl == null) {
+      // The table containing the primary keys is not yet loaded in cache
+      return rawStore.getPrimaryKeys(catName, dbName, tblName);
+    }
+    List<SQLPrimaryKey> keys = sharedCache.listCachedPrimaryKeys(catName, dbName, tblName);

Review comment:
       I think, we should handle the case where table object is cached but constraints are not cached. Now, it seems, we just return empty key list to caller but ideally, we should invoke rawStore.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
##########
@@ -514,6 +655,130 @@ public boolean containsPartition(List<String> partVals) {
       return containsPart;
     }
 
+    public void removeConstraint(String name) {
+      try {
+        tableLock.writeLock().lock();
+        Object constraint = null;
+        MemberName mn = null;
+        Class constraintClass = null;
+        if (this.primaryKeyCache.containsKey(name)) {
+          constraint = this.primaryKeyCache.remove(name);
+          mn = MemberName.PRIMARY_KEY_CACHE;
+          this.memberCacheDirty[mn.ordinal()].set(true);
+          constraintClass = SQLPrimaryKey.class;
+        } else if (this.foreignKeyCache.containsKey(name)) {
+          constraint = this.foreignKeyCache.remove(name);
+          mn = MemberName.FOREIGN_KEY_CACHE;
+          this.memberCacheDirty[mn.ordinal()].set(true);
+          constraintClass = SQLForeignKey.class;
+        } else if (this.notNullConstraintCache.containsKey(name)) {
+          constraint = this.notNullConstraintCache.remove(name);
+          mn = MemberName.NOTNULL_CONSTRAINT_CACHE;
+          this.memberCacheDirty[mn.ordinal()].set(true);
+          constraintClass = SQLNotNullConstraint.class;
+        } else if (this.uniqueConstraintCache.containsKey(name)) {
+          constraint = this.uniqueConstraintCache.remove(name);
+          mn = MemberName.UNIQUE_CONSTRAINT_CACHE;
+          this.memberCacheDirty[mn.ordinal()].set(true);
+          constraintClass = SQLUniqueConstraint.class;
+        }
+
+        if(constraint == null) {
+          LOG.debug("Constraint: " + name + " does not exist in cache.");
+          return;
+        }
+        int size = getObjectSize(constraintClass, constraint);
+        updateMemberSize(mn, -1 * size, SizeMode.Delta);
+
+      } finally {
+        tableLock.writeLock().unlock();
+      }
+    }
+
+    public void refreshPrimaryKeys(List<SQLPrimaryKey> keys) {
+      Map<String, SQLPrimaryKey> newKeys = new ConcurrentHashMap<>();
+      try {
+        tableLock.writeLock().lock();
+        int size = 0;
+        for (SQLPrimaryKey key : keys) {
+          if (this.memberCacheDirty[MemberName.PRIMARY_KEY_CACHE.ordinal()].compareAndSet(true, false)) {

Review comment:
       What is the significance of this check? memberCacheDirty flag is confusing. Pls add a comment describing the meaning of this value and how read/write to cache behave for true or false. If true means, cache is already set, then pls rename it to give correct meaning. It sounds like true means invalid cache.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
##########
@@ -514,6 +655,130 @@ public boolean containsPartition(List<String> partVals) {
       return containsPart;
     }
 
+    public void removeConstraint(String name) {
+      try {
+        tableLock.writeLock().lock();
+        Object constraint = null;
+        MemberName mn = null;
+        Class constraintClass = null;
+        if (this.primaryKeyCache.containsKey(name)) {

Review comment:
       Check if we need to normalize the case of "name" before using for search. it might be coming from user input.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
##########
@@ -514,6 +655,130 @@ public boolean containsPartition(List<String> partVals) {
       return containsPart;
     }
 
+    public void removeConstraint(String name) {
+      try {
+        tableLock.writeLock().lock();
+        Object constraint = null;
+        MemberName mn = null;
+        Class constraintClass = null;
+        if (this.primaryKeyCache.containsKey(name)) {
+          constraint = this.primaryKeyCache.remove(name);
+          mn = MemberName.PRIMARY_KEY_CACHE;
+          this.memberCacheDirty[mn.ordinal()].set(true);
+          constraintClass = SQLPrimaryKey.class;
+        } else if (this.foreignKeyCache.containsKey(name)) {
+          constraint = this.foreignKeyCache.remove(name);
+          mn = MemberName.FOREIGN_KEY_CACHE;
+          this.memberCacheDirty[mn.ordinal()].set(true);
+          constraintClass = SQLForeignKey.class;
+        } else if (this.notNullConstraintCache.containsKey(name)) {
+          constraint = this.notNullConstraintCache.remove(name);
+          mn = MemberName.NOTNULL_CONSTRAINT_CACHE;
+          this.memberCacheDirty[mn.ordinal()].set(true);
+          constraintClass = SQLNotNullConstraint.class;
+        } else if (this.uniqueConstraintCache.containsKey(name)) {
+          constraint = this.uniqueConstraintCache.remove(name);
+          mn = MemberName.UNIQUE_CONSTRAINT_CACHE;
+          this.memberCacheDirty[mn.ordinal()].set(true);
+          constraintClass = SQLUniqueConstraint.class;
+        }
+
+        if(constraint == null) {
+          LOG.debug("Constraint: " + name + " does not exist in cache.");
+          return;
+        }
+        int size = getObjectSize(constraintClass, constraint);
+        updateMemberSize(mn, -1 * size, SizeMode.Delta);
+

Review comment:
       Extra blank line.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
##########
@@ -1434,6 +1699,35 @@ public boolean populateTableInCache(Table table, ColumnStatistics tableColStats,
     tblWrapper.isTableColStatsCacheDirty.set(false);
     tblWrapper.isPartitionColStatsCacheDirty.set(false);
     tblWrapper.isAggrPartitionColStatsCacheDirty.set(false);
+
+    if (cacheObjects.getPrimaryKeys() != null) {
+      if(!tblWrapper.cachePrimaryKeys(cacheObjects.getPrimaryKeys(), true)) {
+        return false;
+      }
+    }
+    tblWrapper.memberCacheDirty[MemberName.PRIMARY_KEY_CACHE.ordinal()].set(false);
+
+    if (cacheObjects.getForeignKeys() != null) {
+      if(!tblWrapper.cacheForeignKeys(cacheObjects.getForeignKeys(), true)) {
+        return false;
+      }
+    }
+    tblWrapper.memberCacheDirty[MemberName.PRIMARY_KEY_CACHE.ordinal()].set(false);

Review comment:
       Member name is incorrect. Check other places below.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
##########
@@ -514,6 +655,130 @@ public boolean containsPartition(List<String> partVals) {
       return containsPart;
     }
 
+    public void removeConstraint(String name) {
+      try {
+        tableLock.writeLock().lock();
+        Object constraint = null;
+        MemberName mn = null;
+        Class constraintClass = null;
+        if (this.primaryKeyCache.containsKey(name)) {
+          constraint = this.primaryKeyCache.remove(name);
+          mn = MemberName.PRIMARY_KEY_CACHE;
+          this.memberCacheDirty[mn.ordinal()].set(true);
+          constraintClass = SQLPrimaryKey.class;
+        } else if (this.foreignKeyCache.containsKey(name)) {
+          constraint = this.foreignKeyCache.remove(name);
+          mn = MemberName.FOREIGN_KEY_CACHE;
+          this.memberCacheDirty[mn.ordinal()].set(true);
+          constraintClass = SQLForeignKey.class;
+        } else if (this.notNullConstraintCache.containsKey(name)) {
+          constraint = this.notNullConstraintCache.remove(name);
+          mn = MemberName.NOTNULL_CONSTRAINT_CACHE;
+          this.memberCacheDirty[mn.ordinal()].set(true);
+          constraintClass = SQLNotNullConstraint.class;
+        } else if (this.uniqueConstraintCache.containsKey(name)) {
+          constraint = this.uniqueConstraintCache.remove(name);
+          mn = MemberName.UNIQUE_CONSTRAINT_CACHE;
+          this.memberCacheDirty[mn.ordinal()].set(true);
+          constraintClass = SQLUniqueConstraint.class;
+        }
+
+        if(constraint == null) {
+          LOG.debug("Constraint: " + name + " does not exist in cache.");
+          return;
+        }
+        int size = getObjectSize(constraintClass, constraint);
+        updateMemberSize(mn, -1 * size, SizeMode.Delta);
+
+      } finally {
+        tableLock.writeLock().unlock();
+      }
+    }
+
+    public void refreshPrimaryKeys(List<SQLPrimaryKey> keys) {
+      Map<String, SQLPrimaryKey> newKeys = new ConcurrentHashMap<>();
+      try {
+        tableLock.writeLock().lock();
+        int size = 0;
+        for (SQLPrimaryKey key : keys) {
+          if (this.memberCacheDirty[MemberName.PRIMARY_KEY_CACHE.ordinal()].compareAndSet(true, false)) {
+            LOG.debug("Skipping primary key cache update for table: " + getTable().getTableName()
+                    + "; the primary keys we have is dirty.");
+            return;
+          }
+          newKeys.put(key.getPk_name(), key);
+          size += getObjectSize(SQLPrimaryKey.class, key);
+        }
+        primaryKeyCache = newKeys;
+        updateMemberSize(MemberName.PRIMARY_KEY_CACHE, size, SizeMode.Snapshot);

Review comment:
       Add a debug log to mark the cache refresh is successful.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
##########
@@ -543,10 +557,30 @@ static void prewarm(RawStore rawStore) {
                 tableColStats = rawStore.getTableColumnStatistics(catName, dbName, tblName, colNames, CacheUtils.HIVE_ENGINE);
                 Deadline.stopTimer();
               }
+              Deadline.startTimer("getPrimaryKeys");
+              primaryKeys = rawStore.getPrimaryKeys(catName, dbName, tblName);
+              Deadline.stopTimer();
+              cacheObjects.setPrimaryKeys(primaryKeys);
+
+              Deadline.startTimer("getForeignKeys");
+              foreignKeys = rawStore.getForeignKeys(catName, null, null, dbName, tblName);

Review comment:
       We need to check if the caller queries the foreign key constraints using parentDb and table as input. If yes, it make sense to map it against parent table object rather than foreign table.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
##########
@@ -1788,6 +2082,58 @@ public void addPartitionToCache(String catName, String dbName, String tblName, P
     }
   }
 
+  public void addPrimaryKeysToCache(String catName, String dbName, String tblName, List<SQLPrimaryKey> keys) {
+    try {
+      cacheLock.readLock().lock();
+      String tblKey = CacheUtils.buildTableKey(catName, dbName, tblName);
+      TableWrapper tblWrapper = tableCache.getIfPresent(tblKey);
+      if (tblWrapper != null) {
+        tblWrapper.cachePrimaryKeys(keys, false);
+      }
+    } finally {
+      cacheLock.readLock().unlock();
+    }
+  }
+
+  public void addForeignKeysToCache(String catName, String dbName, String tblName, List<SQLForeignKey> keys) {
+    try {
+      cacheLock.readLock().lock();
+      String tblKey = CacheUtils.buildTableKey(catName, dbName, tblName);
+      TableWrapper tblWrapper = tableCache.getIfPresent(tblKey);
+      if (tblWrapper != null) {
+        tblWrapper.cacheForeignKeys(keys, false);
+      }
+    } finally {
+      cacheLock.readLock().unlock();
+    }
+  }
+
+  public void addUniqueConstraintsToCache(String catName, String dbName, String tblName, List<SQLUniqueConstraint> keys) {
+    try {
+      cacheLock.readLock().lock();
+      String tblKey = CacheUtils.buildTableKey(catName, dbName, tblName);
+      TableWrapper tblWrapper = tableCache.getIfPresent(tblKey);
+      if (tblWrapper != null) {
+        tblWrapper.cacheUniqueConstraints(keys, false);
+      }
+    } finally {
+      cacheLock.readLock().unlock();
+    }
+  }
+
+  public void addNotNullConstraintsToCache(String catName, String dbName, String tblName, List<SQLNotNullConstraint> keys) {
+    try {
+      cacheLock.readLock().lock();
+      String tblKey = CacheUtils.buildTableKey(catName, dbName, tblName);
+      TableWrapper tblWrapper = tableCache.getIfPresent(tblKey);
+      if (tblWrapper != null) {
+        tblWrapper.cacheNotNulConstraints(keys, false);

Review comment:
       Method name cacheNotNulConstraints missing one "l". :)

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
##########
@@ -1870,6 +2228,122 @@ public void removePartitionsFromCache(String catName, String dbName, String tblN
     return parts;
   }
 
+  public List<SQLPrimaryKey> listCachedPrimaryKeys(String catName, String dbName, String tblName) {
+    List<SQLPrimaryKey> keys = new ArrayList<>();
+    try {
+      cacheLock.readLock().lock();
+      TableWrapper tblWrapper = tableCache.getIfPresent(CacheUtils.buildTableKey(catName, dbName, tblName));
+      if (tblWrapper != null) {
+        keys = tblWrapper.getPrimaryKeys();
+      }
+    } finally {
+      cacheLock.readLock().unlock();
+    }
+    return keys;
+  }
+
+  public List<SQLForeignKey> listCachedForeignKeys(String catName, String foreignDbName, String foreignTblName,
+                                                   String parentDbName, String parentTblName) {
+    List<SQLForeignKey> keys = new ArrayList<>();
+    try {
+      cacheLock.readLock().lock();
+      TableWrapper tblWrapper = tableCache.getIfPresent(CacheUtils.buildTableKey(catName, foreignDbName, foreignTblName));
+      if (tblWrapper != null) {
+        keys = tblWrapper.getForeignKeys();
+      }
+    } finally {
+      cacheLock.readLock().unlock();
+    }
+
+    // filter out required foreign keys based on parent db/tbl name
+    if (!StringUtils.isEmpty(parentTblName) && !StringUtils.isEmpty(parentDbName)) {

Review comment:
       Shall move this if block under if (tblWrapper != null) block above.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
##########
@@ -514,6 +655,130 @@ public boolean containsPartition(List<String> partVals) {
       return containsPart;
     }
 
+    public void removeConstraint(String name) {
+      try {
+        tableLock.writeLock().lock();
+        Object constraint = null;
+        MemberName mn = null;
+        Class constraintClass = null;
+        if (this.primaryKeyCache.containsKey(name)) {
+          constraint = this.primaryKeyCache.remove(name);
+          mn = MemberName.PRIMARY_KEY_CACHE;
+          this.memberCacheDirty[mn.ordinal()].set(true);
+          constraintClass = SQLPrimaryKey.class;
+        } else if (this.foreignKeyCache.containsKey(name)) {
+          constraint = this.foreignKeyCache.remove(name);
+          mn = MemberName.FOREIGN_KEY_CACHE;
+          this.memberCacheDirty[mn.ordinal()].set(true);
+          constraintClass = SQLForeignKey.class;
+        } else if (this.notNullConstraintCache.containsKey(name)) {
+          constraint = this.notNullConstraintCache.remove(name);
+          mn = MemberName.NOTNULL_CONSTRAINT_CACHE;
+          this.memberCacheDirty[mn.ordinal()].set(true);
+          constraintClass = SQLNotNullConstraint.class;
+        } else if (this.uniqueConstraintCache.containsKey(name)) {
+          constraint = this.uniqueConstraintCache.remove(name);
+          mn = MemberName.UNIQUE_CONSTRAINT_CACHE;
+          this.memberCacheDirty[mn.ordinal()].set(true);
+          constraintClass = SQLUniqueConstraint.class;
+        }
+
+        if(constraint == null) {
+          LOG.debug("Constraint: " + name + " does not exist in cache.");
+          return;
+        }
+        int size = getObjectSize(constraintClass, constraint);
+        updateMemberSize(mn, -1 * size, SizeMode.Delta);
+
+      } finally {
+        tableLock.writeLock().unlock();
+      }
+    }
+
+    public void refreshPrimaryKeys(List<SQLPrimaryKey> keys) {
+      Map<String, SQLPrimaryKey> newKeys = new ConcurrentHashMap<>();
+      try {
+        tableLock.writeLock().lock();
+        int size = 0;
+        for (SQLPrimaryKey key : keys) {
+          if (this.memberCacheDirty[MemberName.PRIMARY_KEY_CACHE.ordinal()].compareAndSet(true, false)) {

Review comment:
       We can directly use MemberName.PRIMARY_KEY_CACHE if we assign 0 to first member in the enum.




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

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


Issue Time Tracking
-------------------

    Worklog Id:     (was: 453958)
    Time Spent: 1h 10m  (was: 1h)

> [CachedStore] Cache table constraints in CachedStore
> ----------------------------------------------------
>
>                 Key: HIVE-22015
>                 URL: https://issues.apache.org/jira/browse/HIVE-22015
>             Project: Hive
>          Issue Type: Sub-task
>            Reporter: Daniel Dai
>            Assignee: Adesh Kumar Rao
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 1h 10m
>  Remaining Estimate: 0h
>
> Currently table constraints are not cached. Hive will pull all constraints from tables involved in query, which results multiple db reads (including get_primary_keys, get_foreign_keys, get_unique_constraints, etc). The effort to cache this is small as it's just another table component.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)