hemantk-12 commented on code in PR #4525:
URL: https://github.com/apache/ozone/pull/4525#discussion_r1156675122
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java:
##########
@@ -180,6 +180,18 @@ public OMClientResponse
validateAndUpdateCache(OzoneManager ozoneManager,
omClientResponse = new OMSnapshotCreateResponse(
omResponse.build(), snapshotInfo);
} catch (IOException ex) {
+ // Remove snapshot from the SnapshotChainManager in case of any failure.
+ // It is possible that createSnapshot request fails after snapshot gets
+ // added to snapshot chain manager because couldn't add it to cache/DB.
+ // In that scenario, SnapshotChainManager#globalSnapshotId will point to
+ // failed createSnapshot request's snapshotId but in actual it doesn't
+ // exist in the SnapshotInfo table.
+ // If it doesn't get removed, OM restart will crash on
+ // SnapshotChainManager#loadFromSnapshotInfoTable because it could not
+ // find the previous snapshot which doesn't exist because it was never
+ // added to the SnapshotInfo table.
+ removeSnapshotInfoFromSnapshotChainManager(snapshotChainManager,
Review Comment:
`this.snapshotId == snapshotChainManager.getLatestGlobalSnapshot()` is not
needed as such because it won't be in snapshotChain it was not added and we
have a check
[here](https://github.com/apache/ozone/blob/dc646f55a95b9a7e27a13b228f916ad1534d4edb/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotChainManager.java#L147).
But I see it can be use as short-circuit so added it as suggested.
--
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]