gianm commented on code in PR #18696:
URL: https://github.com/apache/druid/pull/18696#discussion_r2462371639
##########
server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java:
##########
@@ -747,21 +768,30 @@ private SegmentCacheEntry assignLocationAndMount(
}
/**
- * Deletes a directory and logs about it. This method should only be called
under the lock of a {@link #segmentLocks}
+ * Performs an atomic move to a sibling {@link #DROP_PATH} directory, and
then deletes the directory and logs about
+ * it. This method should only be called under the lock of a {@link
#segmentLocks}.
*/
- private static void deleteCacheEntryDirectory(final File path)
+ private static void atomicMoveAndDeleteCacheEntryDirectory(final File path)
{
- log.info("Deleting directory[%s]", path);
+ final File parent = path.getParentFile();
+ final File tempLocation = new File(parent, DROP_PATH);
try {
- FileUtils.deleteDirectory(path);
+ if (!tempLocation.exists()) {
+ FileUtils.mkdirp(tempLocation);
+ }
+ final File tempPath = new File(tempLocation, path.getName());
+ log.debug("moving[%s] to temp location[%s]", path, tempLocation);
+ Files.move(path.toPath(), tempPath.toPath(),
StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING);
Review Comment:
REPLACE_EXISTING doesn't work for directories, but it might be a no-op. At
any rate you might as well remove it.
##########
server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java:
##########
@@ -771,7 +801,13 @@ private static void cleanupLegacyCacheLocation(final File
baseFile, final File c
return;
}
- deleteCacheEntryDirectory(cacheFile);
+ try {
+ log.info("Deleting directory[%s]", cacheFile);
+ FileUtils.deleteDirectory(cacheFile);
+ }
+ catch (Exception e) {
+ log.error(e, "Unable to remove directory[%s]", cacheFile);
Review Comment:
should probably be `warn`, doesn't seem serious enough for `error`
--
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]
