hudi-agent commented on code in PR #20147:
URL: https://github.com/apache/hudi/pull/20147#discussion_r4139376523
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/embedded/EmbeddedTimelineService.java:
##########
@@ -226,43 +225,71 @@ public FileSystemViewStorageConfig
getRemoteFileSystemViewConfig(HoodieWriteConf
}
/**
- * Adds a new base path to the set that are managed by this instance.
- * @param basePath the new base path to add
+ * Acquires a reference for the given base path. Must be called while
holding SERVICE_LOCK.
+ * @param basePath the base path to acquire
*/
private void addBasePath(String basePath) {
- basePaths.add(basePath);
+ basePathReferenceCounts.merge(basePath, 1, Integer::sum);
}
/**
- * Stops the embedded timeline service for the given base path. If a
timeline service is managing multiple tables, it will only be shutdown once all
tables have been stopped.
- * @param basePath For the table to stop the service for
+ * Releases one acquisition for the given base path. The table's view is
cleared only after its last
+ * reference is released, and the service is closed only after all
references have been released.
+ * Each owner must release at most once per acquisition; borrowed services
must not be released.
Review Comment:
🤖 With reference counts, every owner now has to release exactly once.
`HoodieCatalogUtil.alterTable` never closes the reuse-enabled client it gets
from `createWriteClientV2`, so it would now keep the shared service alive after
the other owners close. `StreamSync.close()` can also run twice without
clearing `embeddedTimelineService`. Could you close the catalog client with
try-with-resources and reset that field, here or in a follow-up?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
