Skip to content

branch-4.0: [fix](filecache) avoid recursive empty dir cleanup#63728

Open
liaoxin01 wants to merge 1 commit into
apache:branch-4.0from
liaoxin01:pick-pr-63344-to-branch-4.0
Open

branch-4.0: [fix](filecache) avoid recursive empty dir cleanup#63728
liaoxin01 wants to merge 1 commit into
apache:branch-4.0from
liaoxin01:pick-pr-63344-to-branch-4.0

Conversation

@liaoxin01
Copy link
Copy Markdown
Contributor

@liaoxin01 liaoxin01 commented May 27, 2026

Pick #63344

Copilot AI review requested due to automatic review settings May 27, 2026 07:07
@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?

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates local file cache cleanup to avoid recursively deleting cache directories, using an empty-directory-only removal path instead.

Changes:

  • Adds LocalFileSystem::delete_empty_directory() implemented with rmdir.
  • Uses empty-directory deletion during file cache block removal.
  • Adds unit coverage for deleting empty vs non-empty local directories.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
be/src/io/fs/local_file_system.h Declares the empty-directory deletion API.
be/src/io/fs/local_file_system.cpp Implements non-recursive empty directory removal.
be/src/io/cache/fs_file_cache_storage.cpp Switches file cache cleanup to delete directories only when empty.
be/test/io/fs/local_file_system_test.cpp Adds tests for empty and non-empty directory deletion behavior.

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

@liaoxin01
Copy link
Copy Markdown
Contributor Author

run buildall

@hello-stephen
Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 86.96% (20/23) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 53.21% (19392/36441)
Line Coverage 36.33% (181043/498329)
Region Coverage 32.90% (140483/427036)
Branch Coverage 33.81% (60857/179983)

@hello-stephen
Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 95.65% (22/23) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 71.56% (25527/35673)
Line Coverage 54.36% (270382/497431)
Region Coverage 51.82% (223519/431350)
Branch Coverage 53.31% (96287/180606)

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