[
https://issues.apache.org/jira/browse/HDDS-11452?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18129737#comment-18129737
]
Wei-Chiu Chuang commented on HDDS-11452:
----------------------------------------
Source review of the current implementation at
d0949f291791c399f3e54409fcb92656a80597a4: the core request-level atomicity
concern remains valid. I would keep this issue open and prioritize
fault-injection/recovery coverage.
Evidence:
# OMSnapshotPurgeRequest.validateAndUpdateCache updates successor snapshot
cache entries, removes the snapshot from the in-memory chain, and adds a
snapshot-info cache tombstone before processing the remaining snapshots in the
same request. A later IOException returns an error response without rolling
back those earlier mutations.
# Failed responses do not apply their intended database mutations:
OMClientResponse.checkAndUpdateDB calls addToDBBatch only for Status.OK. Thus,
the failure path can leave chain/cache state inconsistent with persisted
metadata.
# The existing testNumSnapshotDeletedUnchangedOnPartialPurgeFailure explicitly
demonstrates partial mutation: it purges the latest snapshot, injects a
CodecException when reading the next snapshot, receives INTERNAL_ERROR, and
asserts that the first snapshot is absent from the cache. That test protects
metrics accounting, not rollback.
Source and test links:
* Request mutation sequence:
https://github.com/apache/ozone/blob/d0949f291791c399f3e54409fcb92656a80597a4/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotPurgeRequest.java#L116
* Error response without rollback:
https://github.com/apache/ozone/blob/d0949f291791c399f3e54409fcb92656a80597a4/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotPurgeRequest.java#L169
* Failed-response DB handling:
https://github.com/apache/ozone/blob/d0949f291791c399f3e54409fcb92656a80597a4/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/OMClientResponse.java#L63
* Existing partial-failure test:
https://github.com/apache/ozone/blob/d0949f291791c399f3e54409fcb92656a80597a4/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotPurgeRequestAndResponse.java#L312
There has been some hardening: SnapshotChainManager.deleteSnapshot validates
both global and per-path chains before changing either. This protects an
individual chain deletion, but does not make the enclosing multi-snapshot
request atomic:
*
https://github.com/apache/ozone/blob/d0949f291791c399f3e54409fcb92656a80597a4/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotChainManager.java#L397
Qualification on impact: persistent corruption after restart is a possible
consequence, not an established outcome of every failed purge. Restart rebuilds
the chain from persisted snapshot metadata. The confirmed source-level issue is
partial in-memory/cache mutation after a failed request; demonstrating how
subsequent operations turn this into persistent corruption needs a recovery
test. This review does not establish user-data loss.
Suggested next step: extend the existing partial-failure test to assert
chain/cache/DB consistency after failure, exercise subsequent purge requests,
and restart/reload the chain. The fix should ensure request-wide staging or
rollback rather than only adjusting failure metrics.
This assessment is based on source and existing test inspection; tests were not
rerun during this audit.
> OmSnapshotPurgeRequest is not atomic and can lead to SnapshotChain Corruption
> -----------------------------------------------------------------------------
>
> Key: HDDS-11452
> URL: https://issues.apache.org/jira/browse/HDDS-11452
> Project: Apache Ozone
> Issue Type: Sub-task
> Reporter: Swaminathan Balachandran
> Assignee: Swaminathan Balachandran
> Priority: Major
> Labels: pull-request-available
>
> OmSnapshotPurgeRequest updates the snapshot chain and also updates the cache
> & in case of any failure these changes are not rolled back. In case of
> checked exception thrown(This could be any exception ranging from proto
> exception or any random IOException), the request gobbles up the exception
> and returns an error response. The problem with this is, we have partially
> updated snapshot info table cache which is not coherrent with the snapshot
> chain and all these changes won't be flushed to disk. On restart this could
> lead to all sorts of snapshot chain & snapshot info corruption.
> The proposal here is to make the entire request atomic:
> 1) Update the snapshot chain & maintain the updated snapshot infos in local
> uncommitted space.
> 2) In case of an exception, roll back all deleted snapshots by putting it
> back to the snapshot chain(P.S. this needs to be done in the reverse order of
> removal) & return an error response.
> 3) If no exception is thrown, update the snapshot info table cache.
> 4) Send it to double buffer
> cc: [~hemantk] [~ppogde]
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
