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]