sst_importer: ensure cleanup of temporary files on error#19240
Merged
ti-chi-bot[bot] merged 4 commits intotikv:masterfrom Dec 31, 2025
Merged
sst_importer: ensure cleanup of temporary files on error#19240ti-chi-bot[bot] merged 4 commits intotikv:masterfrom
ti-chi-bot[bot] merged 4 commits intotikv:masterfrom
Conversation
…ary files on error Signed-off-by: Juncen Yu <yujuncen@pingcap.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where temporary SST files could be left behind when downloads fail in the sst_importer component. The fix ensures that cleanup happens on all exit paths (both success and error) by using Rust's defer! macro pattern.
Key Changes:
- Implemented automatic cleanup of temporary files using the
defer!macro to ensure cleanup occurs on both success and error paths - Added a new failpoint
download_files_ext_after_downloadto enable testing of the cleanup behavior when errors occur after downloads complete - Added comprehensive test coverage to verify temporary files are properly cleaned up on error
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| components/sst_importer/src/sst_importer.rs | Modified download_files_ext to use defer! for guaranteed cleanup of temporary files, added failpoint for testing, and included a new test test_download_files_ext_cleanup_temp_files_on_error |
| components/sst_importer/Cargo.toml | Added tikv/failpoints dependency to the failpoints feature to enable failpoint testing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Juncen Yu <yujuncen@pingcap.com>
3pointer
approved these changes
Dec 29, 2025
Leavrth
approved these changes
Dec 29, 2025
Contributor
Signed-off-by: Juncen Yu <yujuncen@pingcap.com>
…download-cleanup Signed-off-by: Juncen Yu <yujuncen@pingcap.com>
iosmanthus
approved these changes
Dec 31, 2025
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 3pointer, iosmanthus, Leavrth The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Contributor
Author
|
/test pull-unit-test |
Member
|
In response to a cherrypick label: new pull request created to branch |
9 tasks
YuJuncen
added a commit
to YuJuncen/tikv
that referenced
this pull request
Dec 31, 2025
ref tikv#17283, close tikv#19239 sst_importer: enhance failpoint handling and ensure cleanup of temporary files on error - Ensure temporary files are cleaned up when a download fails in sst_importer - Add a new failpoint `download_files_ext_after_download` to inject errors right after batch downloads complete, improving test coverage of cleanup paths Signed-off-by: Juncen Yu <yujuncen@pingcap.com>
YuJuncen
added a commit
that referenced
this pull request
Dec 31, 2025
* sst_importer: set fill cache to false when merge downloaded SST files (#19229) close #19228 set fill cache to false when merge downloaded SST files Signed-off-by: Jianjun Liao <jianjun.liao@outlook.com> * sst_importer: ensure cleanup of temporary files on error (#19240) ref #17283, close #19239 sst_importer: enhance failpoint handling and ensure cleanup of temporary files on error - Ensure temporary files are cleaned up when a download fails in sst_importer - Add a new failpoint `download_files_ext_after_download` to inject errors right after batch downloads complete, improving test coverage of cleanup paths Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * Squashed commit of the following: commit 5a14b2b Author: Juncen Yu <yujuncen@pingcap.com> Date: Mon Dec 29 15:11:06 2025 +0800 make clippy happy Signed-off-by: Juncen Yu <yujuncen@pingcap.com> commit ace595e Merge: bd5bff3 147cacc Author: Juncen Yu <yujuncen@pingcap.com> Date: Fri Dec 26 17:51:36 2025 +0800 Merge branch 'master' of https://github.com/tikv/tikv into resolve-compact-conflict commit bd5bff3 Merge: fde14ce 301984c Author: Juncen Yu <yujuncen@pingcap.com> Date: Mon Dec 22 12:10:59 2025 +0800 Merge branch 'master' of https://github.com/tikv/tikv into resolve-compact-conflict Signed-off-by: Juncen Yu <yujuncen@pingcap.com> commit fde14ce Author: Juncen Yu <yujuncen@pingcap.com> Date: Tue Mar 18 15:44:06 2025 +0800 added more checks Signed-off-by: Juncen Yu <yujuncen@pingcap.com> commit d0801cb Author: Juncen Yu <yujuncen@pingcap.com> Date: Tue Mar 18 15:27:14 2025 +0800 added more case about collapsed rollback Signed-off-by: Juncen Yu <yujuncen@pingcap.com> commit 6d2b994 Author: Juncen Yu <yujuncen@pingcap.com> Date: Tue Mar 18 14:52:40 2025 +0800 compact-log-backup: try to resolve conflict encountering during compacting Signed-off-by: Juncen Yu <yujuncen@pingcap.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> * *: upgrade cargo-deny version to make it compatible to further developments. Signed-off-by: lucasliang <nkcs_lykx@hotmail.com> --------- Signed-off-by: Jianjun Liao <jianjun.liao@outlook.com> Signed-off-by: Juncen Yu <yujuncen@pingcap.com> Signed-off-by: lucasliang <nkcs_lykx@hotmail.com> Co-authored-by: Jianjun Liao <36503113+Leavrth@users.noreply.github.com> Co-authored-by: lucasliang <nkcs_lykx@hotmail.com>
ti-chi-bot bot
pushed a commit
that referenced
this pull request
Jan 7, 2026
…9256) ref #17283, close #19239 sst_importer: enhance failpoint handling and ensure cleanup of temporary files on error - Ensure temporary files are cleaned up when a download fails in sst_importer - Add a new failpoint `download_files_ext_after_download` to inject errors right after batch downloads complete, improving test coverage of cleanup paths Signed-off-by: Juncen Yu <yujuncen@pingcap.com> Signed-off-by: 山岚 <36239017+YuJuncen@users.noreply.github.com> Co-authored-by: Juncen Yu <yujuncen@pingcap.com> Co-authored-by: 山岚 <36239017+YuJuncen@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is changed and how it works?
Issue Number: Close #19239, ref #17283
What's Changed:
Related changes
pingcap/docs/pingcap/docs-cn:Check List
Tests
Side effects
Release note