fix: remove recycled log files leaked by unclean shutdown

[waterWang]

Fix #407.

Problem

When a raft-engine process exits without running SinglePipe::drop (SIGKILL, OOM kill, panic=abort), recycled append files keep their original .raftlog name on disk (purge_to intentionally does not rename them; only shutdown renames them to .raftlog.reserved). On restart they are re-scanned as regular append files, the hole check drops them from the file list (files.drain(..invalid_idx)), but the physical files are never deleted — they become unreachable and leak on disk forever, accumulating across unclean restarts.

Fix

When the hole check drains the invalid file prefix, also delete the corresponding physical log files (for the append/rewrite queues). Recycled/reserved files keep the existing no-delete behavior. Handles are closed before deletion so the fix also works on Windows.

Test

test_remove_recycled_files_left_by_unclean_shutdown (in src/engine.rs): writes data, purges to populate the recycle pool, performs a clean reopen, then simulates an unclean shutdown by renaming the .raftlog.reserved files back to plain .raftlog files (reproducing the leaked on-disk state). After reopening, it asserts that every plain .raftlog file on disk belongs to the active span — i.e. no leaked files remain.

Summary by CodeRabbit

• Bug Fixes
  • Improved recovery after unclean shutdowns by removing leaked log files during engine restart.
  • Enhanced detection and cleanup of missing or duplicate log-file sequences.
  • Preserved recycled files while safely deleting invalid non-recycled files from disk.
• Tests
  • Added coverage for cleanup of files left behind after an unclean shutdown.

[ti-chi-bot]

Hi @waterWang. Thanks for your PR.

I'm waiting for a tikv member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[ti-chi-bot]

Welcome @waterWang!

It looks like this is your first PR to tikv/raft-engine 🎉.

I'm the bot to help you request reviewers, add labels and more, See available commands.

We want to make sure your contribution gets all the attention it needs!



Thank you, and welcome to tikv/raft-engine. 😃

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c2066ce4-8290-47a5-aee8-ee4eec866f8f

📥 Commits

Reviewing files that changed from the base of the PR and between 8c8624c and 73ca757.

📒 Files selected for processing (2)

• src/engine.rs
• src/file_pipe_log/pipe_builder.rs

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

The append-file scan now deletes leaked non-recycled files that precede sequence holes. A new engine test simulates an unclean shutdown with recycled logs and verifies that restart removes the leaked files.

Changes

Recycled log cleanup

Layer / File(s)	Summary
Restart scan cleanup and validation src/file_pipe_log/pipe_builder.rs, src/engine.rs	The scan closes and deletes invalid non-recycled files, while recycled files are removed from memory. The new test simulates leaked recycled files and verifies their removal and the resulting append-file span.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: 🔵 Low · up to 73ca7

The PR removes abandoned log files during restart, but malformed or duplicate filenames could make cleanup target the wrong canonical path, and filesystem deletion failures can leave stale files while startup continues. The change is mergeable with explicit owner awareness of these bounded recovery risks.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the primary change: removing recycled log files leaked after an unclean shutdown.
Linked Issues check	✅ Passed	The changes address issue #407 by deleting invalid leaked log files during restart cleanup, closing non-recycled file handles before deletion, preserving reserved-file behavior, and adding a regressio…
Out of Scope Changes check	✅ Passed	The changes are limited to restart cleanup logic and its regression test. No unrelated code changes are identified.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.

Full details: Linked Issues check

Explanation

The changes address issue #407 by deleting invalid leaked log files during restart cleanup, closing non-recycled file handles before deletion, preserving reserved-file behavior, and adding a regression test.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Connor1996]

LGTM

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Connor1996, LykxSassinator

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [Connor1996,LykxSassinator]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ti-chi-bot]

[LGTM Timeline notifier]

Timeline:

• 2026-08-27 23:13:55.102865803 +0000 UTC m=+802670.273959930: ☑️ agreed by Connor1996.
• 2026-08-28 02:50:24.815931022 +0000 UTC m=+815659.987025150: ☑️ agreed by LykxSassinator.

[LykxSassinator]

Thx

[ti-chi-bot]

@waterWang: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
rust-nightly	73ca757	link	true	/test rust-nightly

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[Manphil]

Why skip recycled files here? Don't .reserved files leak the same way?

[ti-chi-bot]

@Manphil: adding LGTM is restricted to approvers and reviewers in OWNERS files.

Details

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.