Skip to content

Conversation

@swamirishi
Copy link
Contributor

What changes were proposed in this pull request?

Exceptions handling refactoring in the snapshot code.

Please describe your PR in detail:
There are quite a few places where the snapshot code just throw new RuntimeException, the patch proposes to add a custom exception to handle exceptions better.

What is the link to the Apache JIRA

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

How was this patch tested?

Refactoring changes, existing testcases and clean build should be sufficient.

@swamirishi swamirishi added the snapshot https://issues.apache.org/jira/browse/HDDS-6517 label Jan 8, 2024
@swamirishi swamirishi requested a review from aswinshakil January 8, 2024 19:17
@smengcl
Copy link
Contributor

smengcl commented Jan 17, 2024

Thanks @swamirishi for taking care of this! When would this be out of draft?

Copy link
Contributor

@hemantk-12 hemantk-12 left a comment

Choose a reason for hiding this comment

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

Thanks @swamirishi for working on this.

I think it is unnecessary to wrap exception to SnapshotException and SnapshotRuntimeException. Simpler approach would be to use directly use IOException and RuntimeException. For all the customer facing code path, use IOException or OMException. And for background services and jobs (e.g. SnapshotDiff jobs, SSTFilteringService, SnapshotDeletingService, etc.) use RuntimeException.

Also in SnapshotErrors, half of the error codes are internal to Ozone and don't think it would make any sense to expose them to user. If it is needed, may be use generic snapshot error code. And rest of error codes are in ResultCodes. We should reuse existing one when converting IOException to OMException.

@adoroszlai
Copy link
Contributor

/pending conflicts

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Marking this issue as un-mergeable as requested.

Please use /ready comment when it's resolved.

Please note that the PR will be closed after 21 days of inactivity from now. (But can be re-opened anytime later...)

conflicts

@github-actions
Copy link

Thank you very much for the patch. I am closing this PR temporarily as there was no activity recently and it is waiting for response from its author.

It doesn't mean that this PR is not important or ignored: feel free to reopen the PR at any time.

It only means that attention of committers is not required. We prefer to keep the review queue clean. This ensures PRs in need of review are more visible, which results in faster feedback for all PRs.

If you need ANY help to finish this PR, please contact the community on the mailing list or the slack channel."

@github-actions github-actions bot closed this Dec 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pending snapshot https://issues.apache.org/jira/browse/HDDS-6517

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants