Skip to content

atomic move segment files to staging location prior to delete#18696

Merged
clintropolis merged 3 commits into
apache:masterfrom
clintropolis:atomic-ish-delete
Oct 27, 2025
Merged

atomic move segment files to staging location prior to delete#18696
clintropolis merged 3 commits into
apache:masterfrom
clintropolis:atomic-ish-delete

Conversation

@clintropolis

Copy link
Copy Markdown
Member

This PR changes how segment files are deleted from the cache by introducing an atomic move to a staging directory before performing the delete. This ensures that there will not be partial segment files hanging out in the event of a failure during a delete operation, and the staging directory is cleaned up on process start.

The staging directory is in the root of the storage location, so if the segment cache path for a segment is something like

.../segment_cache_folder/test_2019-01-01T00:00:00.000Z_2019-01-02T00:00:00.000Z_2025-10-24T23:44:38.503Z_5

it would be moved to

.../segment_cache_folder/__drop/test_2019-01-01T00:00:00.000Z_2019-01-02T00:00:00.000Z_2025-10-24T23:44:38.503Z_5

prior to deleting.

… no partial segment files if a failure during a delete operation occurs
Comment thread server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java Dismissed
@github-actions github-actions Bot added Area - Batch Ingestion Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 labels Oct 25, 2025
}
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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REPLACE_EXISTING doesn't work for directories, but it might be a no-op. At any rate you might as well remove it.

FileUtils.deleteDirectory(cacheFile);
}
catch (Exception e) {
log.error(e, "Unable to remove directory[%s]", cacheFile);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably be warn, doesn't seem serious enough for error

Comment thread server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java Dismissed
Comment thread server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java Dismissed
@cryptoe

cryptoe commented Oct 27, 2025

Copy link
Copy Markdown
Contributor

@clintropolis Is this a Druid 35 blocker ?

@clintropolis
clintropolis merged commit 0be30ef into apache:master Oct 27, 2025
91 of 92 checks passed
@clintropolis
clintropolis deleted the atomic-ish-delete branch October 27, 2025 18:08
@kgyrtkirk kgyrtkirk added this to the 36.0.0 milestone Jan 19, 2026
riovic918data pushed a commit to riovic918data/druid that referenced this pull request Jun 12, 2026
…#18696)

* atomic move segment files to staging location prior to drop to ensure no partial segment files if a failure during a delete operation occurs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area - Batch Ingestion Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants