smengcl commented on code in PR #11706:
URL: https://github.com/apache/ozone/pull/11706#discussion_r4419686311
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java:
##########
@@ -81,24 +83,30 @@ public static synchronized ContainerCache getInstance(
}
/**
- * Closes all the db instances and resets the cache.
+ * Closes and removes all the db instances that are not in use.
*/
public void shutdownCache() {
+ List<String> inUse = new ArrayList<>();
lock.lock();
try {
// iterate the cache and close each db
MapIterator iterator = cache.mapIterator();
while (iterator.hasNext()) {
iterator.next();
ReferenceCountedDB db = (ReferenceCountedDB) iterator.getValue();
- Preconditions.checkArgument(cleanupDb(db), "refCount: %s",
- db.getReferenceCount());
+ if (cleanupDb(db)) {
+ iterator.remove();
+ } else {
+ // Still referenced: leave it open for its user; it is closed once
released and evicted or removed.
Review Comment:
> If shutdownCache() is called only once and no later eviction/removal
happens, the DB remains open after its user finishes.
Correct. It stays open until it is evicted, removed through `removeDB` (for
example when its container is deleted), or the process exits.
> Please clarify whether that lifetime is intentional, and whether the
shutdown caller guarantees a later cleanup pass.
It is intentional, and the caller does not guarantee a later pass. The only
production caller is `KeyValueHandler#stop` during datanode stop, which is
followed by JVM exit (the shutdown hook on SIGTERM, or `terminate(1)` in
`terminateDatanode`, which this patch makes reachable again). Before this patch
the referenced DB, and every DB after it in the iteration, was left open the
same way.
> The new test demonstrates eventual closure by calling shutdownCache() a
second time, but that is an explicit retry rather than automatic cleanup.
Yes. The second call only checks that a skipped entry stays in the cache and
can still be closed once released, instead of being dropped from the cache and
leaked. Closing it automatically on final release is left to the follow-up on
the JIRA.
--
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]
