Siyao Meng created HDDS-17672:
---------------------------------
Summary: 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
Reporter: Siyao Meng
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]
