feat: Clean up deep storage after index parallel merge finishes by GWphua · Pull Request #19187 · apache/druid

GWphua · 2026-03-20T07:09:14Z

Fixes #19163

Description

In the context of multi-phase index_parallel tasks, intermediary files created onto deep storage are not being cleaned up.

Fixed the issue by triggering deepstore intermediary-data cleanup from the supervisor task after the merge in phase 2 completes.

This fix only works for HDFS.

Release note

Introduced a mechanism to automatically clean up intermediary files on HDFS Storage.

Key changed/added classes in this PR

ParallelIndexSupervisorTask

This PR has:

been self-reviewed.
added documentation for new or modified features or behaviors.
a release note entry in the PR description.
added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
added or updated version, license, or notice information in licenses.yaml
added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
added integration tests.
been tested in a test Druid cluster.

Add delete for datasegments Unit Tests and Embedded tests Cleanup

FrankChen021 · 2026-03-20T13:02:20Z

+   *                may be null or empty if phase 1 produced no output.
+   */
+  @VisibleForTesting
+  static void cleanupDeepStorageShuffleData(


is the dir deleted?

Depends on the implementation of the DataSegmentKiller.

From what I see, all file storage implementations like LocalDataSegmentKiller, HdfsDataSegmentKiller and AzureDataSegmentKiller will remove the dir if it is empty.

I did a test on HDFS. Here's some findings:

The HDFS shuffle-data are stored in somewhere like the following:

segments/shuffle-data/<taskId>/<startInterval>/<endInterval>/<partition>/<PartialIndexGeneratorTaskId>/0_index.zip

Current implementation of kill in HdfsDataSegmentKiller hard-codes removal depth of 2 to 3, meaning we will still see EMPTY directories of the following after all segments are killed:

-- Before -- segments/shuffle-data/<taskId>/<startInterval>/<endInterval>/<partition>/<PartialIndexGeneratorTaskId>/0_index.zip -- After -- segments/shuffle-data/<taskId>/<startInterval>/<endInterval> (Deleted 2 layers up)

I will add a new method in DataSegmentKiller interface solely for handling intermediary tasks, and implementations will default to the kill method. Users using other storage forms are welcome to implement their own methods.

GWphua · 2026-03-24T03:59:54Z

Latest commit tested and works in cluster with HDFS implementation.

GWphua · 2026-03-25T09:32:04Z

@FrankChen021

Thanks for the review, I have made the appropriate changes.

FrankChen021

LGTM.

FrankChen021 · 2026-03-25T14:38:09Z

Also requested review from copilot to see if there's anything I missed

Copilot

Pull request overview

This PR addresses leaked deep-storage intermediary (“shuffle-data”) artifacts produced by multi-phase index_parallel ingestion by adding a best-effort recursive delete from the supervisor task’s cleanup path, with support currently implemented for HDFS.

Changes:

Added DataSegmentKiller#killRecursively(relativePath) (default no-op) and implemented delegation in OmniDataSegmentKiller.
Implemented recursive directory deletion in HdfsDataSegmentKiller and invoked it from ParallelIndexSupervisorTask#cleanUp for shuffle-data/<supervisorTaskId>.
Added/updated unit tests and updated documentation to reflect HDFS-only automated cleanup.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`processing/src/main/java/org/apache/druid/segment/loading/DataSegmentKiller.java`	Adds `killRecursively` API (default no-op) for deep-storage prefix cleanup.
`server/src/main/java/org/apache/druid/segment/loading/OmniDataSegmentKiller.java`	Delegates `killRecursively` to all registered killers and aggregates IOExceptions.
`server/src/test/java/org/apache/druid/segment/loading/OmniDataSegmentKillerTest.java`	Verifies `killRecursively` delegation behavior.
`indexing-service/src/main/java/org/apache/druid/indexing/worker/shuffle/DeepStorageIntermediaryDataManager.java`	Adds helper to compute the `shuffle-data/<supervisorTaskId>` prefix and clarifies cleanup responsibility.
`indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/ParallelIndexSupervisorTask.java`	Calls deep-storage recursive cleanup during supervisor `cleanUp`, swallowing IOExceptions.
`indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/ParallelIndexSupervisorTaskTest.java`	Adds tests around cleanup invoking `killRecursively` and handling failures.
`extensions-core/hdfs-storage/src/main/java/org/apache/druid/storage/hdfs/HdfsDataSegmentKiller.java`	Implements `killRecursively` with path validation and recursive HDFS delete.
`extensions-core/hdfs-storage/src/test/java/org/apache/druid/storage/hdfs/HdfsDataSegmentKillerTest.java`	Adds tests for `killRecursively`, including validation and colon normalization.
`docs/configuration/index.md`	Updates config docs to state automated shuffle-data cleanup is supported for HDFS deep storage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <[email protected]>

jtuglu1 · 2026-03-26T04:21:41Z

This fix only works for HDFS.

Is there a reason we cannot extend to generic storage? Does the killRecursively need to be implemented on the storage type for this to be supported in other storage backends?

Also, is this limited to index_parallel? What about compaction, etc.

GWphua · 2026-03-26T06:05:28Z

We are only using HDFS as deep store for our Druid clusters. Hence, I can only verify the behaviour for HDFS-based storages.

For other storages, creating a new PR to implement killRecursively will help to solve this issue.

Also, is this limited to index_parallel? What about compaction, etc.

This problem is only limited to multi-phase index_parallel tasks.

jtuglu1 · 2026-03-26T23:50:13Z

We are only using HDFS as deep store for our Druid clusters. Hence, I can only verify the behaviour for HDFS-based storages.

For other storages, creating a new PR to implement killRecursively will help to solve this issue.

Also, is this limited to index_parallel? What about compaction, etc.

This problem is only limited to multi-phase index_parallel tasks.

I believe I have seen this also with compaction tasks which perform shuffle for perfect rollup.

GWphua · 2026-03-27T02:39:56Z

For native compaction task: tasks will also create shuffle directories. If you look at how ParallelIndexSupervisorTask#cleanUp works, it will try to clean up the shuffle directory before the isCompactionTask check.

GWphua added 10 commits March 18, 2026 17:00

Shuffle solution

bd24bcc

Add delete for datasegments Unit Tests and Embedded tests Cleanup

Better task logs when fail killing.

54723c2

Docs

4e72f17

Merge remote-tracking branch 'origin/master' into clean-up-deepstore

72432d4

Checkstyle

b8a3cc3

Cleanup function

55d3db8

non-volatile

2393411

Clean up unnecessary code

220049b

Test fixes

d63c8b0

Merge remote-tracking branch 'origin/master' into clean-up-deepstore

d41f302

github-actions Bot added Area - Documentation Area - Ingestion labels Mar 20, 2026

FrankChen021 reviewed Mar 20, 2026

View reviewed changes

GWphua added 2 commits March 23, 2026 17:11

HDFS Segment clean-up

24aae13

Finish up deepstore

a93156c

github-actions Bot added the Area - Segment Format and Ser/De label Mar 23, 2026

Fix HDFS pathing bug

ff91fdc

GWphua changed the title ~~Clean up deep storage after index parallel merge finishes~~ feat: Clean up deep storage after index parallel merge finishes Mar 23, 2026

GWphua added 2 commits March 23, 2026 19:09

Fix forbidden apis

5f11ad9

Deep Store PR

1a5027a

Tests

fa7b9de

GWphua mentioned this pull request Mar 24, 2026

Parallel indexing with deepstore intermediary data does not clean up shuffle-data in deep storage #19163

Closed

FrankChen021 reviewed Mar 24, 2026

View reviewed changes

Comment thread docs/configuration/index.md Outdated

Comment thread processing/src/main/java/org/apache/druid/segment/loading/DataSegmentKiller.java Outdated

GWphua added 6 commits March 25, 2026 10:11

Correct Docs

8624b83

Change DataSegmentKiller to not be aware of supervisor shuffle

52887f1

Test unhappy path

5632f33

Exception type

493749b

Checkstyle

39d8e17

Extra space in md file

2bd449d

GWphua added 4 commits March 25, 2026 11:23

Forbidden API

e19eb86

No need to check taskId as AbstractTask already did

95fc295

Rename Shuffle method

813c398

Merge remote-tracking branch 'origin/master' into clean-up-deepstore

ffe1288

FrankChen021 requested a review from Copilot March 25, 2026 14:37

FrankChen021 approved these changes Mar 25, 2026

View reviewed changes

Copilot started reviewing on behalf of FrankChen021 March 25, 2026 14:38 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

GWphua and others added 3 commits March 26, 2026 10:15

Split method that preserves empty tokens

b3dff0b

Update docs/configuration/index.md

07f0012

Co-authored-by: Copilot <[email protected]>

Address co-pilot comments

aec3504

jtuglu1 requested review from jtuglu1 March 26, 2026 23:49

GWphua merged commit 3d1e0d0 into apache:master Mar 30, 2026
63 of 64 checks passed

github-actions Bot added this to the 37.0.0 milestone Mar 30, 2026

kfaraz mentioned this pull request Jul 2, 2026

fix: Perform cleanup of intermediary data safely #19650

Open

10 tasks

Uh oh!

Conversation

GWphua commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Release note

Key changed/added classes in this PR

Uh oh!

FrankChen021 Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

GWphua Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

GWphua Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GWphua commented Mar 24, 2026

Uh oh!

Uh oh!

Uh oh!

GWphua commented Mar 25, 2026

Uh oh!

FrankChen021 left a comment

Choose a reason for hiding this comment

Uh oh!

FrankChen021 commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jtuglu1 commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GWphua commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jtuglu1 commented Mar 26, 2026

Uh oh!

GWphua commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

GWphua commented Mar 20, 2026 •

edited

Loading

GWphua Mar 23, 2026 •

edited

Loading

jtuglu1 commented Mar 26, 2026 •

edited

Loading

GWphua commented Mar 26, 2026 •

edited

Loading

GWphua commented Mar 27, 2026 •

edited

Loading