HDDS-13160. Remove SnapshotDirectoryCleaningService and refactor AbstractDeletingService

[swamirishi]

What changes were proposed in this pull request?

After HDDS-13034 SnapshotDirectoryCleaningService shouldn't be used anywhere, thus remove all traces of the class and also move respective functions in AbstractDeletingService to respective background services and just keeping the common ones.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-13160

How was this patch tested?

No Tests required code refactoring.

[swamirishi]

Review by Co-Pilot:
Based on the diff you provided from the PR (https://github.com/apache/ozone/pull/8547/files), here is a thorough analysis of all functions moved from AbstractKeyDeletingService to DirectoryDeletingService and KeyDeletingService, with a line-by-line check for any changes:

---

1. Functions moved to DirectoryDeletingService

a. optimizeDirDeletesAndSubmitRequest

• Moved to: DirectoryDeletingService (was previously in AbstractKeyDeletingService)
• Diff/Change:
  The code is copied as-is. No logic change.
  • Parameters, variable names, and logic remain identical.
  • No line differences detected.
• Conclusion: No change except location.

---

b. prepareDeleteDirRequest

• Moved to: DirectoryDeletingService
• Diff/Change:
  Copied as-is.
  • All steps (step-0 to step-3), variables, and return conditions are identical.
• Conclusion: No change except location.

---

c. wrapPurgeRequest

• Moved to: DirectoryDeletingService
• Diff/Change:
  No logic change.
  • Still returns a built PurgePathRequest using the same parameters and logic.
• Conclusion: No change except location.

---

d. submitPurgePaths

• Moved to: DirectoryDeletingService
• Diff/Change:
  Only two minor differences:
  • The method is now private (was protected).
  • Uses getClientId().toString() instead of direct field access clientId.toString().
  • All logic, structure, and error handling are otherwise identical.
• Conclusion: Only access modifier and clientId access changed; logic is the same.

---

e. getDeletedDirsCount, getMovedDirsCount, getMovedFilesCount

• Moved to: DirectoryDeletingService
  (Were previously in AbstractKeyDeletingService)
• Diff/Change:
  Method bodies are unchanged—simply return values from atomic counters.
• Conclusion: No change except location.

---

2. Functions moved to KeyDeletingService

a. processKeyDeletes

• Moved to: KeyDeletingService (was previously in AbstractKeyDeletingService)
• Diff/Change:
  Logic is copied as-is:
  • SCM block deletion, timing, logging, and call to submitPurgeKeysRequest are all identical.
  • Only change is the use of getPerfMetrics() accessor instead of a direct field for performance metrics.
• Conclusion: No meaningful logic change.

---

b. submitPurgeKeysRequest

• Moved to: KeyDeletingService
• Diff/Change:
  Logic remains identical:
  • Handles building the PurgeKeysRequest, sets fields, error handling, and return values.
  • Uses getClientId().toString() instead of direct field, as with DirectoryDeletingService.
• Conclusion: Only clientId accessor changed; logic is the same.

---

3. Functions Removed from AbstractKeyDeletingService (Not moved)

• suspend, resume, shouldRun: Removed from both DirectoryDeletingService and KeyDeletingService as part of class cleanup.
  • No new version in the subclasses; these are simply no longer present.

---

Summary Table

Function	Old Location	New Location	Changes?
optimizeDirDeletesAndSubmitRequest	AbstractKeyDeletingService	DirectoryDeletingService	None
prepareDeleteDirRequest	AbstractKeyDeletingService	DirectoryDeletingService	None
wrapPurgeRequest	AbstractKeyDeletingService	DirectoryDeletingService	None
submitPurgePaths	AbstractKeyDeletingService	DirectoryDeletingService	Only access modifier + clientId
getDeletedDirsCount	AbstractKeyDeletingService	DirectoryDeletingService	None
getMovedDirsCount	AbstractKeyDeletingService	DirectoryDeletingService	None
getMovedFilesCount	AbstractKeyDeletingService	DirectoryDeletingService	None
processKeyDeletes	AbstractKeyDeletingService	KeyDeletingService	None (minor accessor change)
submitPurgeKeysRequest	AbstractKeyDeletingService	KeyDeletingService	Only clientId accessor

---

Conclusion

• All functions were moved without logic changes (except minor access modifier and clientId accessor adjustments).
• This refactor was a pure move to make base and subclass responsibilities clearer, with no impact on behavior or output.

If you want a detailed, line-by-line diff for a specific function, let me know the function name!

[jojochuang]

would dirDeletingServiceCorePoolSize needs to be larger than 10 if number of snapshots is big? should we pick a larger default value

[swamirishi]

we can start out with default of 10. Snapshots would be always created lazily. Even if we create one snapshot in a minute. There would be at the most 20-30 snapshots for deep cleaning every 15 minutes rest of the snapshots would be just noop.

[jojochuang]

LGTM.

A few suggestions made from my IDE. They can be dealt with in the future. For now I'd suggest to go ahead and merge it to make the refactor easier.

[jojochuang]

can be a final variable

[jojochuang]

return value not used.

[jojochuang]

can be made final

[swamirishi]

Thank you for reviewing the PR @jojochuang . I will create a follow up jira for the above review comments