Skip to content

branch-4.1: [fix](be) Clean up spill directories during query teardown - #66458

Merged
yiguolei merged 2 commits into
apache:branch-4.1from
mrhhsg:backport/spill-directory-cleanup-branch-4.1
Aug 6, 2026
Merged

branch-4.1: [fix](be) Clean up spill directories during query teardown#66458
yiguolei merged 2 commits into
apache:branch-4.1from
mrhhsg:backport/spill-directory-cleanup-branch-4.1

Conversation

@mrhhsg

@mrhhsg mrhhsg commented Aug 5, 2026

Copy link
Copy Markdown
Member

What problem does this PR solve?

Issue Number: None

Related PR: #66328

Problem Summary:

Backport the spill query-directory cleanup from #66328 to branch-4.1.

This change:

  • records each spill data root before opening the first spill part;
  • deletes touched per-query spill directories during QueryContext teardown;
  • retains failed deletions and retries them from spill GC and shutdown paths;
  • preserves the no-spill fast path without scanning every configured spill root.

The recursive CTE and FragmentMgr lifecycle changes from the original PR are intentionally excluded because the branch-4.1 implementation diverges. Spill cleanup for query contexts retained by that lifecycle remains out of scope for this backport.

Release note

None

Check List (For Author)

  • Test

    • Regression test
      • ASAN FE/BE build with ./build.sh --be --fe succeeded.
      • doris-local-regression run -d spill_p0 -s test_spill_directory_cleanup (1/1 passed, then rerun 1/1 passed).
      • The case confirms real spill writes and observes spill_disk_has_spill_data transition from 0 to 1 and back to 0 after query cleanup.
    • Unit Test
      • ASAN doris_be_test target compiled and linked successfully.
      • ./run-be-ut.sh --run --filter='SpillFileTest.GCCleansUpFiles:SpillFileTest.QueryContextDeletesEmptySpillDirectory:SpillFileTest.QueryContextCleansUpNestedSpillDirectory:SpillFileTest.QueryContextDeletesResidualSpillDirectory:SpillFileTest.QueryContextCleansUpAllTouchedSpillDirectories:SpillFileTest.QueryContextContinuesCleanupAfterRootFailure:SpillFileTest.QueryContextRetriesSpillDirectoryDeletionUntilSuccess:SpillFileTest.RetryPreservesDirectoryQueuedAfterPendingDrain:SpillFileTest.QueryContextSkipsCleanupWithoutSpill:SpillFileTest.DeleteSpillFileThroughManagerSynchronously' -j 64 (10/10 passed, then rerun 10/10 passed)
      • build-support/check-format.sh (passed)
      • git diff --check (passed)
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • Behavior changed:

    • No.
    • Yes. Touched spill query directories are removed during query teardown, and transient deletion failures are retained for retry.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

Backport the spill-directory cleanup from apache#66328:

- record each spill data root before opening the first spill part;
- delete touched per-query spill directories during QueryContext teardown;
- retain failed deletions and retry them from spill GC and shutdown paths;
- keep no-spill queries on the zero-cleanup fast path.

The recursive CTE and FragmentMgr lifecycle changes from the original PR
are intentionally excluded because branch-4.1 has divergent recursive CTE
code. Spill cleanup for that lifecycle remains out of scope.

(cherry picked from commit 3dff55a)
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@mrhhsg

mrhhsg commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

run buildall

@mrhhsg

mrhhsg commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 100.00% (70/70) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 58.26% (24644/42302)
Line Coverage 42.46% (247626/583222)
Region Coverage 38.50% (195529/507872)
Branch Coverage 39.88% (89344/224056)

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100.00% (70/70) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 73.70% (30386/41232)
Line Coverage 57.57% (333247/578851)
Region Coverage 54.48% (277447/509240)
Branch Coverage 55.42% (124037/223814)

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100.00% (70/70) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 73.69% (30384/41232)
Line Coverage 57.57% (333236/578851)
Region Coverage 54.49% (277501/509240)
Branch Coverage 55.41% (124026/223814)

@mrhhsg
mrhhsg marked this pull request as ready for review August 6, 2026 09:21
@mrhhsg
mrhhsg requested a review from yiguolei as a code owner August 6, 2026 09:21
@mrhhsg

mrhhsg commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

/review

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Codex automated review failed and did not complete.

Error: All Codex review accounts are usage-limited; earliest retry is 2026-08-08T03:32:00Z.
Workflow run: https://github.com/apache/doris/actions/runs/31088743648

Please trigger /review again after that time.

@yiguolei
yiguolei merged commit 3ba435c into apache:branch-4.1 Aug 6, 2026
31 of 34 checks passed
@mrhhsg
mrhhsg deleted the backport/spill-directory-cleanup-branch-4.1 branch August 6, 2026 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants