Copilot commented on code in PR #11896:
URL: https://github.com/apache/ozone/pull/11896#discussion_r4437535511


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -2127,9 +2129,15 @@ <KEY, VALUE> Table<KEY, VALUE> 
get(DBColumnFamilyDefinition<KEY, VALUE> definiti
       return get(definition.getTable(store));
     }
 
-    <KEY, VALUE> Table<KEY, VALUE> get(DBColumnFamilyDefinition<KEY, VALUE> 
definition, CacheType cacheType)
+    <KEY, VALUE> Table<KEY, VALUE> get(DBColumnFamilyDefinition<KEY, VALUE> 
definition, CacheType tableCacheType)
         throws IOException {
-      return get(definition.getTable(store, cacheType));
+      // A FULL_CACHE table loads the whole table and starts a cleanup thread 
on every open, so a DB opened with
+      // PARTIAL_CACHE (snapshot and checkpoint DBs, which are opened and 
closed repeatedly) must not have one.
+      if (tableCacheType == CacheType.FULL_CACHE && cacheType != 
CacheType.FULL_CACHE) {
+        throw new IllegalArgumentException("Table " + definition.getName() + " 
must not use " + tableCacheType
+            + " in a DB opened with " + cacheType);

Review Comment:
   This check throws `IllegalArgumentException` after the DB store has already 
been opened. The snapshot constructor only calls `stop()` for `IOException`, 
and the checkpoint constructor has no failure cleanup, so the exact 
misconfiguration this guard is meant to catch leaves the partially initialized 
RocksDB manager/resources open. Please close the manager when initialization 
fails with this runtime exception as well (or otherwise ensure both 
constructors clean up on validation failure).



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to