[
https://issues.apache.org/jira/browse/HDDS-17672?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17672:
------------------------------
Attachment: TestBugCR10UpdateRollbackClearsMetadata.java
> KeyValueContainer.update clears all container metadata instead of restoring
> it when the .container file rewrite fails
> ---------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17672
> URL: https://issues.apache.org/jira/browse/HDDS-17672
> Project: Apache Ozone
> Issue Type: Bug
> Affects Versions: 0.5.0, 1.0.0, 1.1.0, 1.2.0, 1.3.0, 1.2.1, 1.4.0, 2.0.0,
> 1.4.1, 2.1.0, 2.2.0, 2.1.1, 2.1.2, 2.2.1, 2.2.2
> Reporter: Siyao Meng
> Priority: Minor
> Attachments: CR-10-restore-container-metadata-on-failed-update.patch,
> TestBugCR10UpdateRollbackClearsMetadata.java
>
>
> h3. Mechanism
> {{KeyValueContainer.update}} keeps {{containerData.getMetadata()}} as the
> value to restore if the {{.container}} rewrite fails.
> {{ContainerData.getMetadata}} returns
> {{Collections.unmodifiableMap(this.metadata)}}, a live view of the same map,
> not a copy. On failure the {{catch}} calls
> {{containerData.setMetadata(oldMetadata)}}, which runs {{metadata.clear()}}
> and then {{metadata.putAll(oldMetadata)}}. The {{clear()}} also empties the
> view, so nothing is put back: the old keys and the new ones are all gone from
> memory. The next successful {{.container}} write (another update, or a state
> change such as close) persists the empty map.
> HDDS-267 describes the intended behaviour: if the on-disk update of the
> {{.container}} file fails, the in-memory container metadata is reset to the
> old value.
> h3. Trigger
> # An {{UpdateContainer}} request sets metadata on an open container.
> # A later {{UpdateContainer}} request fails to rewrite the {{.container}}
> file (for example a disk fault), returning {{CONTAINER_FILES_CREATE_ERROR}}.
> h3. Impact
> * After step 2 the container has no metadata in memory, so a
> {{ReadContainer}} response would return none (read from
> {{KeyValueContainerData.getProtoBufMessage}}, not run).
> * The next successful {{.container}} write stores only the keys added after
> the failure, so the earlier keys are lost on disk.
> * No current Ozone component sets or reads container metadata: no Ozone code
> sends {{UpdateContainer}} today, and the rollback also needs a failed
> {{.container}} write. The defect therefore has no effect on clusters today;
> it matters for anyone who starts using container metadata.
> * The container import path also calls {{update}}, but it discards the
> imported container on failure, so it is not affected.
> h3. Reproduction
> [^TestBugCR10UpdateRollbackClearsMetadata.java]
> ({{hadoop-hdds/container-service}}) builds a {{KeyValueContainer}} on a
> temporary directory, sets {{volume}} and {{owner}}, then calls {{update}}
> with a metadata path in a directory that does not exist, so creating the
> {{.container}} temp file fails with a real {{IOException}}. It prints
> {{metadata in memory after the failed update: \{\}}} and, after one more
> successful update, {{metadata in .container after the next update:
> \{k3=v3\}}}. A passing test means the defect is present. With the patch it
> fails at the first assertion because the two keys are restored.
> h3. Patch
> [^CR-10-restore-container-metadata-on-failed-update.patch], against
> ec5ed5f7ea0e4b406aee53aae4906616819cba2b. It also applies to master at
> 7294fc2da18.
> {{update}} now takes the write lock first and saves {{new
> HashMap<>(containerData.getMetadata())}} as the rollback copy, so the
> rollback restores the previous entries and the copy is consistent with
> concurrent updates. The success path is unchanged.
> Covered by
> {{TestKeyValueContainer#testFailedUpdateContainerRestoresMetadata}} (all
> layout and schema variants), which checks the in-memory metadata after the
> failed update and the {{.container}} content after the next successful one.
> Without the change it fails in all 8 variants with {{expected:
> \{"owner"="hdfs", "volume"="ozone"\} but was: \{\}}}. With it
> {{TestKeyValueContainer}}, {{TestContainerPersistence}},
> {{TestKeyValueHandler}} and {{TestTarContainerPacker}} pass (659 tests) and
> checkstyle is clean.
> Found by code review of the datanode container state machine apply and block
> commit paths, as part of the TLA+ verification effort under HDDS-15926, on
> commit ec5ed5f7ea0e4b406aee53aae4906616819cba2b. Checked against HDDS issues
> and apache/ozone pull requests for duplicates before filing. The attached
> patch is a proposal for review. Generated with Specula (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
