[GitHub] [iceberg] marton-bod commented on a change in pull request #2547: Hive: add table level JVM lock on HiveCatalog commit operation

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

marton-bod commented on a change in pull request #2547:
URL: https://github.com/apache/iceberg/pull/2547#discussion_r623880632



##########
File path: hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java
##########
@@ -96,6 +102,15 @@
       GC_ENABLED, "external.table.purge"
   );
 
+  private static Cache<String, ReentrantLock> commitLockCache;
+
+  private static synchronized void initTableLevelLockCache(long evictionTimeout) {
+    if (commitLockCache == null) {
+      commitLockCache = Caffeine.newBuilder()
+          .expireAfterAccess(evictionTimeout, TimeUnit.MILLISECONDS)

Review comment:
       `expireAfterAccess` is based on the amount of time that elapsed after the last read/write operation on the cache entry. So the scenario that you mentioned could only happen if the C1 commit takes more than the entire eviction timeout period (10 minutes by default) which is very unlikely. Even if it does happens (e.g. due to some extreme lock starvation), and C2 starts the commit operation, it shouldn't have too much of an impact because the HMS locking mechanism will still enforce that there won't be write conflicts between threads (i.e. we would be basically back to where we are in the status quo, without the table-level locks).




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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org