HDDS-13554. Clean up snapshot local YAML file on Snapshot purge.#8939
Conversation
jojochuang
left a comment
There was a problem hiding this comment.
@SaketaChalamchala can you rebase? I believe the PR contains unrelated parts.
069c47e to
b02351d
Compare
b02351d to
1ddc194
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR ensures cleanup of snapshot local YAML files during snapshot purge operations. The change adds deletion of YAML metadata files that track snapshot compaction information, which were previously left behind when snapshots were purged.
Key changes:
- Enhanced snapshot purge to delete associated YAML metadata files
- Updated tests to verify YAML file cleanup during purge operations
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| OMSnapshotPurgeResponse.java | Added deletion of snapshot local YAML metadata files during checkpoint cleanup |
| TestOMSnapshotPurgeRequestAndResponse.java | Enhanced test coverage to verify YAML file existence before purge and cleanup after purge |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| OmSnapshotManager.getSnapshotLocalPropertyYamlPath(omMetadataManager, snapshotInfo)); | ||
| try { | ||
| FileUtils.deleteDirectory(snapshotDirPath.toFile()); | ||
| Files.deleteIfExists(snapshotLocalDataPath); |
There was a problem hiding this comment.
The YAML file deletion should be included in the same try-catch block as the directory deletion to ensure consistent error handling and logging for both operations.
|
Thanks @SaketaChalamchala |
What changes were proposed in this pull request?
HDDS-13253 Creates a local data file (YAML) on snapshot create or every snapshot to track snapshot compaction metadata.
This PR ensures that the local data file is removed along with the snapshot checkpoint on snapshot purge.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13554
How was this patch tested?
Updated existing Unit test.