Hdds 13760

[priyeshkaratha]

What changes were proposed in this pull request?

Provide a one-liner summary of the changes in the PR Title field above.
It should be in the form of HDDS-1234. Short summary of the change.

Please describe your PR in detail:

• What changes are proposed in the PR? and Why? It would be better if it is written from third person's
  perspective not just for the reviewer.
• Provide as much context and rationale for the pull request as possible. It could be copy-paste from
  the Jira's description if the jira is well defined.
• If it is complex code, describe the approach used to solve the issue. If possible attach design doc,
  issue investigation, github discussion, etc.

Examples of well-written pull requests:

• #3980
• #5265
• #4701
• #5283
• #5300

What is the link to the Apache JIRA

Please create an issue in ASF JIRA before opening a pull request, and you need to set the title of the pull
request which starts with the corresponding JIRA issue number. (e.g. HDDS-XXXX. Fix a typo in YYY.)

(Please replace this section with the link to the Apache JIRA)

How was this patch tested?

(Please explain how this patch was tested. Ex: unit tests, manual tests, workflow run on the fork git repo.)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this.)

[gemini-code-assist]

Summary of Changes

Hello @priyeshkaratha, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a critical synchronization mechanism for Ozone Manager's background key and directory deleting services. It ensures that these services do not initiate a new purge operation on the Active Object Store (AOS) until all changes from the preceding purge have been fully flushed to disk. This is achieved by adding new metrics to track the last purged transaction ID and implementing a check within the deleting services to compare this ID against the flushed transaction ID. Additionally, a minor optimization is included in the key gathering logic.

Highlights

• Intent: This pull request aims to enhance the robustness and consistency of the background key and directory deleting services in Ozone Manager. Specifically, it introduces a mechanism to prevent these services from starting a new purge cycle on the Active Object Store (AOS) until the changes from the previous purge operation have been fully flushed to disk. This ensures that the deleting services operate on a consistent state of the metadata.
• Changes: 1. Metrics Update: New metrics (lastAOSPurgeTermId, lastAOSPurgeTransactionId) are added to DeletingServiceMetrics.java to track the transaction ID of the last successful purge operation on the Active Object Store (AOS). Corresponding getter and setter methods (getLastAOSTransactionInfo, setLastAOSTransactionInfo) are also introduced.

2. Transaction Recording: OMDirectoriesPurgeRequestWithFSO.java and OMKeyPurgeRequest.java are modified to update these new metrics with the current transaction's term and index whenever a purge operation is executed on the AOS.
3. Service Synchronization: A new method isPreviousPurgeTransactionFlushed() is added to AbstractKeyDeletingService.java. This method checks if the transaction ID of the last recorded purge (from the new metrics) is greater than the transaction ID that has been flushed to disk. If it is, it indicates that the previous purge's changes are not yet persistent.
4. Conditional Execution: DirectoryDeletingService.java and KeyDeletingService.java are updated to call isPreviousPurgeTransactionFlushed() before processing AOS deletions. If the previous purge is not yet flushed, the services will skip the current AOS processing cycle.
5. Minor Optimization: KeyManagerImpl.java sees a small optimization in gatherSubPathsWithIterator by removing redundant checks for immediate children and existence, as these are deemed unnecessary when the directory deleting service runs.

• Reviewer Activity: No reviewer activity has been recorded for this pull request yet.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

I've reviewed the changes and the overall approach to prevent the deleting service from running before a previous purge transaction is flushed seems solid. The introduction of lastAOSPurgeTermId and lastAOSPurgeTransactionId metrics, along with the logic in isPreviousPurgeTransactionFlushed, correctly addresses the problem. The refactoring in KeyManagerImpl to simplify sub-path gathering is a good cleanup. The changes are well-contained and consistently applied across the relevant services. I don't have any major concerns with this pull request. Great work!