You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "hemantk-12 (via GitHub)" <gi...@apache.org> on 2023/09/08 18:28:50 UTC

[GitHub] [ozone] hemantk-12 opened a new pull request, #5262: HDDS-9199. Snapshot chain corruption should not fail OM restart

hemantk-12 opened a new pull request, #5262:
URL: https://github.com/apache/ozone/pull/5262

   ## What changes were proposed in this pull request?
   Currently, OM restart fails and leaves the OM in unstable (which can't be stabilized without manual effort) if there is any kind of failure in snapshot chain reconstruction. Previous issues ([HDDS-7689](https://issues.apache.org/jira/browse/HDDS-7689), [HDDS-8530](https://issues.apache.org/jira/browse/HDDS-8530), [HDDS-8832](https://issues.apache.org/jira/browse/HDDS-8832) and [HDDS-9073](https://issues.apache.org/jira/browse/HDDS-9073)) where OM restart failed and left it in unstable state.
   
   In this change, it is proposed to keep the snapshot chain state and let the OM start without any snapshot/chain related issues. Snapshot chain corrupted state is used to block all the snapshot's write operations like create, delete and purge snapshot if snapshot chain is corrupted.
   
   A new parameter, `snapshotChainCorrupted`, is added to `SnapshotChainManager` to keep the state if chain is loaded successfully or not. It is used in access methods `addSnapshot()` to chain, `deleteSnapshot()` from chain and few others before returning response to the caller. In case of chain corruption, `addSnapshot()`, `deleteSnapshot()` will throw `IllegalStateException` and fail snapshot write operations.
   
   ## What is the link to the Apache JIRA
   https://issues.apache.org/jira/browse/HDDS-9199
   
   ## How was this patch tested?
   Updated unit and integration tests.
   


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-9199. Snapshot chain corruption should not fail OM restart [ozone]

Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on PR #5262:
URL: https://github.com/apache/ozone/pull/5262#issuecomment-1747348299

   Thanks @smengcl and @swamirishi for the review.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-9199. Snapshot chain corruption should not fail OM restart [ozone]

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #5262:
URL: https://github.com/apache/ozone/pull/5262#discussion_r1345886082


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java:
##########
@@ -298,7 +298,7 @@ private void removeSnapshotInfoFromSnapshotChainManager(
   ) {
     try {
       snapshotChainManager.deleteSnapshot(info);
-    } catch (IOException exception) {
+    } catch (IllegalStateException exception) {
       LOG.error("Failed to remove snapshot: {} from SnapshotChainManager.",
           info, exception);
     }

Review Comment:
   Is it intended that this exception is swallowed here?



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-9199. Snapshot chain corruption should not fail OM restart [ozone]

Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 merged PR #5262:
URL: https://github.com/apache/ozone/pull/5262


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-9199. Snapshot chain corruption should not fail OM restart [ozone]

Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5262:
URL: https://github.com/apache/ozone/pull/5262#discussion_r1346155808


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java:
##########
@@ -298,7 +298,7 @@ private void removeSnapshotInfoFromSnapshotChainManager(
   ) {
     try {
       snapshotChainManager.deleteSnapshot(info);
-    } catch (IOException exception) {
+    } catch (IllegalStateException exception) {
       LOG.error("Failed to remove snapshot: {} from SnapshotChainManager.",
           info, exception);
     }

Review Comment:
   Yes, because we will be throwing actual exception caused the snapshot creation here: https://github.com/apache/ozone/blob/719500a659cc6ef00746f554bee3f3fbfd6558b2/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java#L281. 



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-9199. Snapshot chain corruption should not fail OM restart [ozone]

Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on PR #5262:
URL: https://github.com/apache/ozone/pull/5262#issuecomment-1747342797

   > I guess we can just disable snapshots all together in case of snapshot corruption. OzoneManager#isFilesystemSnapshotEnabled(). Since snapdiff & deep clean should also not run.
   
   1. `OzoneManager#isFilesystemSnapshotEnabled()` is based on ozone config. I don't think it is a good idea to leverage it for snapshot chain corruption. We can add something else but it would be similar to `SnapshotChainManager#snapshotChainCorrupted`.
   
   2. Any operation relying on `SnapshotChainManager` should fail and would fail. If I'm not wrong, snapDiff, snapshot read and snapshot list don't rely on `SnapshotChainManager`. So I think it is OK to let them go. Regarding deep clean (or create snapshot), depends on  `SnapshotChainManager` because it is used to move keys around and cause snapshot corruption is wrong keys are moved. Deep clean will fail here: https://github.com/apache/ozone/blob/master/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java#L228 and similar for creation.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] umamaheswararao commented on pull request #5262: HDDS-9199. Snapshot chain corruption should not fail OM restart

Posted by "umamaheswararao (via GitHub)" <gi...@apache.org>.
umamaheswararao commented on PR #5262:
URL: https://github.com/apache/ozone/pull/5262#issuecomment-1723924639

   @swamirishi Could you also please take a look at it?


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org