Skip to content

HDDS-12650. Added logs to SnapshotDeletingService to indicate skipped snapshots. #8123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hemantk-12 merged 5 commits into from
Mar 25, 2025
Merged

HDDS-12650. Added logs to SnapshotDeletingService to indicate skipped snapshots. #8123

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
690609d
HDDS-12650. Added logs to SnapshotDeletingService to indicate skipped…
Mar 20, 2025
72df2fe
HDDS-12650. Addressed comments.
Mar 21, 2025
cb38067
Fixed log message.
hemantk-12 Mar 21, 2025
1b34eac
Revert "Fixed log message."
Mar 21, 2025
ca20ad6
Merge branch 'apache:master' into HDDS-12650
SaketaChalamchala Mar 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions ...one-manager/src/main/java/org/apache/hadoop/ozone/om/service/SnapshotDeletingService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,13 +173,18 @@ public BackgroundTaskResult call() throws InterruptedException {
// another.
if (nextSnapshot != null &&
nextSnapshot.getSnapshotStatus() != SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE) {
LOG.info("Skipping Snapshot Deletion processing for : {}", snapInfo.getTableKey());
Copy link
Contributor

@hemantk-12 hemantk-12 Mar 20, 2025

Choose a reason for hiding this comment

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

  1. Move line 170 to before line 167.
  2. Add a log statement for lines 167-169 with the message Skipping Snapshot Deletion processing because the snapshot is not active or DB changes are not flushed: {}?
  3. Change the message to Skipping Snapshot Deletion processing for : {} because the next snapshot is DELETED..

Copy link
Contributor Author

@SaketaChalamchala SaketaChalamchala Mar 21, 2025

Choose a reason for hiding this comment

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

@hemantk-12,
2. Should this log be Skipping Snapshot Deletion processing because the snapshot is active or DB changes are not flushed: {}? and do we want to add this even as a debug log? Wouldn't it be cleaner for SnapshotDeletingService to log the progress of snapshots marked for deletion only? In which case, 1. might also not be necessary.

Otherwise, everytime SnapshotDeletingService runs we will see logs for all snapshots, most of which will say "not deleting this snapshot because it is active" which may not indicate anything.

Copy link
Contributor

@hemantk-12 hemantk-12 Mar 21, 2025

Choose a reason for hiding this comment

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

Sure, we can add debug logs for points 2 and 3.

continue;
}

// nextSnapshot = null means entries would be moved to AOS.
if (nextSnapshot == null) {
LOG.info("Snapshot: {} entries will be moved to AOS.", snapInfo.getTableKey());
waitForKeyDeletingService();
waitForDirDeletingService();
} else {
LOG.info("Snapshot: {} entries will be moved to next active snapshot: {}",
snapInfo.getTableKey(), nextSnapshot.getTableKey());
}
try (ReferenceCounted<OmSnapshot> snapshot = omSnapshotManager.getSnapshot(
snapInfo.getVolumeName(), snapInfo.getBucketName(), snapInfo.getName())) {
Expand Down