swamirishi commented on PR #8547: URL: https://github.com/apache/ozone/pull/8547#issuecomment-2955035876
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! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
