[Feature]: Remove empty directory after gc the entire partition dir

[thweetkomputer]

Feature Summary

Motivation

Proposed Solution

[liunyl]

Looking into code

[liunyl]

GC Local/Cloud Path Cleanup Implementation Plan

Overview

Refactor local-path cleanup so that local mode and cloud mode have distinct owners:

• Local mode keeps current manifest deletion owned by EvictRootForCache() -> CleanManifest().
• Local GC only deletes unreferenced local data_* files and then opportunistically removes an empty or .tmp-only partition directory.
• Cloud GC continues to delete only remote objects. After remote deletion succeeds, it queues the corresponding local cache files into FileCleaner, and FileCleaner becomes the only owner of cloud local-file and local-directory cleanup.

Current State Analysis

• WriteTask::TriggerFileGC() directly dispatches local GC or cloud GC based on cloud_store_path, so the two modes already have separate execution paths. See src/tasks/write_task.cpp:659.
• Local GC currently deletes only local unreferenced data_* files through DeleteUnreferencedLocalFiles(). It does not own current manifest deletion. See src/file_gc.cpp:47 and src/file_gc.cpp:540.
• Local manifest cleanup is currently gated by RootMetaMgr::EvictRootForCache(): it requires ref_cnt_ == 0, no pinned index pages, and MappingCount() == 0 && manifest_size_ > 0 before calling IoMgr()->CleanManifest(tbl_id). See src/storage/root_meta.cpp:263.
• IouringMgr::CleanManifest() deletes the current manifest and then attempts to remove the partition directory. See src/async_io_manager.cpp:884.
• Cloud GC currently deletes only remote objects by submitting ObjectStore::DeleteTask for cloud paths. It does not delete local cached files. See src/file_gc.cpp:627 and src/file_gc.cpp:436.
• Cloud local-file eviction is currently owned by CloudStoreMgr::FileCleaner, and closed local cache files enter the cleaner via CloudStoreMgr::CloseFile() -> EnqueClosedFile(). See src/async_io_manager.cpp:4293 and src/async_io_manager.cpp:4879.
• CloudStoreMgr::CleanManifest() is empty today, so cloud mode has no local manifest/path cleanup on the EvictRootForCache() path. See src/async_io_manager.cpp:4102.

Desired End State

• Local mode:
  • ExecuteLocalGC() deletes unreferenced local data_* files exactly as it does today.
  • ExecuteLocalGC() may remove the local partition directory only when the directory is empty or contains only stale .tmp files.
  • ExecuteLocalGC() does not delete the current manifest.
  • CleanManifest() remains the only owner of current manifest deletion and performs the final directory cleanup after manifest removal.
• Cloud mode:
  • ExecuteCloudGC() deletes only remote objects.
  • After remote deletion succeeds, ExecuteCloudGC() passes the deleted basenames to CloudStoreMgr.
  • CloudStoreMgr::FileCleaner deletes the corresponding local cached files when they become deletable and removes the local partition directory if it becomes empty or .tmp-only.
  • Cloud GC itself never deletes local files or local directories directly.

Out of Scope

• No change to the existing retained-file calculation in BuildRetainedFiles().
• No change to remote object deletion semantics or remote prefix cleanup behavior.
• No restart-time full reconciliation between local cache contents and cloud contents.
• No change to archive retention or archive parsing rules.

Implementation Approach

Phase 1: Shared Local Directory Cleanup Primitive

Goal

Create one low-level helper that can safely remove a local leaf partition directory after files have already been deleted by the correct owner.

Changes

• Add an internal IouringMgr helper, for example TryCleanupLocalPartitionDir(const TableIdent &tbl_id), declared in include/async_io_manager.h and implemented in src/async_io_manager.cpp.
• Helper behavior is fixed:
  • Resolve store_path/<table>.<partition>.
  • If the directory does not exist, return KvError::NoError.
  • Probe GetOpenedFD(tbl_id, LruFD::kDirectory) and GetOpenedFD(tbl_id, LruFD::kManifest).
  • Treat ref_count_ > 1 after probing as active use and skip cleanup.
  • Treat ref_count_ == 1 as cached-idle and close that FD before attempting deletion.
  • Scan the directory with std::filesystem plus std::error_code.
  • If any non-.tmp entry exists, leave the directory in place.
  • If only .tmp entries remain, delete those .tmp entries first.
  • Remove the leaf directory with UnlinkAt(GetRootFD(tbl_id), tbl_id.ToString().c_str(), true).
  • Treat -ENOENT and -ENOTEMPTY as benign concurrent outcomes.
• Use UnlinkAt() for directory deletion and file deletion inside the helper; do not introduce a new batch delete API for directories.

Success Criteria

• The helper is reusable from local GC, CleanManifest(), and cloud FileCleaner.
• Active kDirectory or kManifest references block cleanup.
• Cached-but-idle kDirectory and kManifest handles do not keep empty directories alive forever.

Phase 2: Local Mode Ownership Cleanup

Goal

Keep local-mode ownership clean: local GC owns data-file cleanup; CleanManifest() owns current manifest cleanup.

Changes

• In src/file_gc.cpp, keep DeleteUnreferencedLocalFiles() logic unchanged.
• After DeleteUnreferencedLocalFiles() succeeds in ExecuteLocalGC(), call io_mgr->TryCleanupLocalPartitionDir(tbl_id).
• Because the helper only removes empty or .tmp-only directories, local GC will not delete a current manifest or a directory that still contains one.
• In src/async_io_manager.cpp, refactor IouringMgr::CleanManifest() to:
  • Keep its current manifest unlink logic.
  • Replace its inline rmdir logic with a call to TryCleanupLocalPartitionDir(tbl_id).
• Do not add current-manifest deletion to ExecuteLocalGC().

Success Criteria

• Local GC can remove directories only after the manifest is already gone.
• EvictRootForCache() -> CleanManifest() remains the only path that deletes the current manifest in local mode.
• Local cleanup logic is not duplicated between ExecuteLocalGC() and CleanManifest().

Phase 3: Cloud Remote-GC-Driven Local Cleanup

Goal

Make cloud GC trigger targeted local cache cleanup without taking ownership of local deletion itself.

Changes

• Change DeleteUnreferencedCloudFiles() in src/file_gc.cpp to return the successfully deleted cloud basenames in addition to its status.
• Include all remote files that were actually deleted:
  • Unreferenced data_* files.
  • The current manifest when the existing cloud GC rule decides to delete it.
  • Old-term manifests.
• Add an internal CloudStoreMgr API in include/async_io_manager.h, for example RequestGcLocalCleanup(const TableIdent &tbl_id, const std::vector<std::string> &filenames).
• Add two new CloudStoreMgr members in include/async_io_manager.h:
  • A pending set keyed by FileKey for deduplication.
  • A pending queue of FileKey work items for FileCleaner.
• RequestGcLocalCleanup() must:
  • Convert basenames to FileKey.
  • Insert unseen keys into the pending set and queue.
  • Call file_cleaner_.Resume() if the cleaner is idle.
• Update ExecuteCloudGC() to:
  • Keep remote deletion behavior unchanged.
  • Call cloud_mgr->RequestGcLocalCleanup(tbl_id, deleted_filenames) after remote deletion succeeds.
  • Never delete local files or directories directly.

FileCleaner Behavior

• CloudStoreMgr::FileCleaner::Run() must process pending GC cleanup before normal cache-pressure eviction.
• The cleaner idle condition must also require the pending GC-cleanup queue to be empty.
• For each pending FileKey, the cleaner must:
  • Check whether the file has an opened FD.
  • If an opened FD exists and ref_count_ > 1, leave the key pending and retry later.
  • If an opened FD exists and ref_count_ == 1, close the cached-idle FD so the file can become a closed cache file.
  • Look for the file in closed_files_.
  • If present there, unlink the local file through io_uring, update used_local_space_, remove it from the LRU/closed_files_, and clear the pending key.
  • If the file is already absent locally, clear the pending key.
• CloudStoreMgr::CloseFile() must wake FileCleaner when it closes a file whose FileKey is still in the pending GC-cleanup set. This guarantees that a file which was open during cloud GC gets deleted soon after the last close.
• After each batch of successful local deletions, FileCleaner must deduplicate touched partitions and call TryCleanupLocalPartitionDir(tbl_id) for each one.

Success Criteria

• Cloud GC still only owns remote deletion.
• Cloud local cache cleanup is deterministic and targeted, not deferred until cache pressure.
• A remote-GC’d local file that is still open is not deleted immediately, but is deleted after it becomes idle.
• Local partition directory removal in cloud mode happens only from FileCleaner.

Test Plan

tests/gc.cpp

• Add a local-mode test where GC deletes the last local data_* file and the directory is already manifest-free; verify the directory is removed.
• Add a local-mode test where the directory contains only .tmp after GC; verify .tmp and directory are removed.
• Add a local-mode test for EvictRootForCache() -> CleanManifest() where manifest deletion is followed by directory removal.

tests/cloud.cpp

• Add a cloud test where remote GC deletes files and FileCleaner subsequently deletes the matching local cached files.
• Add a cloud test where the last locally cached file of a partition is removed by FileCleaner and the local partition directory disappears afterward.
• Add a cloud test where the file is still open when remote GC completes; verify the local file survives until close and is then cleaned up.
• Add a cloud test where the directory becomes .tmp-only and FileCleaner removes the stale .tmp plus the directory.

Verification Style

• Cloud cleanup assertions must poll for eventual success because FileCleaner runs asynchronously.
• Local cleanup assertions may be synchronous for CleanManifest() and request-driven for explicit local GC.
• All new tests must verify both behavior and ownership:
  • Local current manifest is not deleted by local GC.
  • Cloud local path deletion does not happen directly inside ExecuteCloudGC().

Risks and Guardrails

• Do not let local GC delete the current manifest. That would overlap with CleanManifest() and bypass the stronger root-eviction gate in RootMetaMgr::EvictRootForCache().
• Do not let cloud GC delete local files directly. That would conflict with the existing closed_files_ / FileCleaner ownership model.
• Treat only kDirectory and kManifest as directory-cleanup blockers. Data-file cleanup in cloud mode is already serialized through the targeted FileKey pending set and FileCleaner.
• Keep all directory deletion idempotent and tolerant of concurrent ENOENT / ENOTEMPTY.

Final Acceptance Criteria

• Local mode:
  • Unreferenced data_* files are still deleted by local GC.
  • Current manifest deletion remains exclusively on the CleanManifest() path.
  • Empty or .tmp-only local partition directories are eventually removed.
• Cloud mode:
  • Remote GC deletes only remote objects.
  • The exact local cache files corresponding to remote-GC’d objects are eventually deleted by FileCleaner.
  • Local partition directories are removed only by FileCleaner, after the local directory becomes empty or .tmp-only.

[liunyl]

Implementation completed.
Further testing and review will be done next week