[
https://issues.apache.org/jira/browse/HDDS-17227?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17227:
------------------------------
Attachment: TestBugCR6DatanodeIdLostOnFailedWrite.java
> A failed rewrite of datanode.id leaves it empty, so the datanode loses its
> operational state and initial version on the next start
> ----------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17227
> URL: https://issues.apache.org/jira/browse/HDDS-17227
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments: CR-6-replace-datanode-id-atomically.patch,
> TestBugCR6DatanodeIdLostOnFailedWrite.java
>
>
> h3. Mechanism
> {{ContainerUtils.writeDatanodeDetailsTo}} deletes an existing {{datanode.id}}
> and creates it again empty before {{DatanodeIdYaml.createDatanodeIdFile}}
> writes the new content through {{YamlUtils.dump}}. That helper writes to a
> temporary file with Ratis {{AtomicFileOutputStream}} and moves it over the
> target when the stream is closed. Two things defeat the atomic write:
> * the delete: any write that does not complete leaves an empty file, or no
> file if the process dies between the delete and the create;
> * {{YamlUtils.dump}} closes the stream in a try with resources also when
> {{yaml.dump}} throws, so after a failed write, for example on a full disk,
> the empty or partial temporary file is still moved over the target. The
> stream's {{abort()}} is never called.
> On the next start {{ContainerUtils.readDatanodeDetailsFrom}} finds the empty
> file and recovers only the UUID from the volume VERSION file (HDDS-13955).
> The operational state becomes IN_SERVICE, its expiry 0, the certificate
> serial ID null and the initial version {{DEFAULT_VERSION}}. Hostname and
> software version are set again on every start, so their loss has no effect.
> h3. Trigger
> No crash is needed, a write that fails while the datanode keeps running is
> enough:
> * an I/O error while the content is written, for example a full metadata disk
> ({{File too large}} under a file size limit was run as well);
> * an I/O error when the temporary file is created;
> * an interrupted command thread: {{DatanodeStateMachine.close}} interrupts
> the thread that runs {{SetNodeOperationalStateCommandHandler}}, and the write
> fails with {{ClosedByInterruptException}}.
> The file is rewritten through this method for every
> {{SetNodeOperationalStateCommand}}, on every datanode start
> ({{InitDatanodeState}}) and on certificate rotation in secure mode
> ({{HddsDatanodeService.saveNewCertId}}). The missing file case needs a crash
> between two consecutive system calls and was not run.
> h3. Impact
> From 2.2.0 the datanode starts after an empty {{datanode.id}}, with these
> losses:
> * The initial version does not heal: it is only set when {{datanode.id}} is
> created, and {{XceiverServerRatis.assignPorts}} uses the separate Ratis
> server and admin ports only for an initial version that supports them. After
> one such start the datanode serves Raft on its client port (9858 by default)
> instead of the server port (9856) for good. By code reading, existing
> pipelines whose Raft groups hold the old server address then cannot reach
> this datanode until SCM closes them; not run on a cluster.
> * The persisted operational state is IN_SERVICE with no maintenance end time.
> A stable leader SCM still holds the state and sends it again on the next
> heartbeat. By code reading the state is lost for good when no SCM holds it at
> that point, for example after all SCMs restarted, or when leadership moves to
> a follower that adopted the reported IN_SERVICE before the correction reached
> the datanode; not run.
> * The certificate serial ID in the file is lost.
> In 2.0.x and 2.1.x, which have the atomic write (HDDS-12531, first released
> in 2.0.0) but not the recovery (HDDS-13955), an empty {{datanode.id}} stops
> the datanode from starting with the {{NullPointerException}} quoted in
> HDDS-13955 until an operator repairs the file. Releases before 2.0.0 wrote
> the file in place and emptied it on a failed write as well. No container data
> is lost.
> h3. Reproduction
> PASS on unmodified source, deterministic.
> [^TestBugCR6DatanodeIdLostOnFailedWrite.java]
> ({{hadoop-hdds/container-service}}) writes {{datanode.id}} with
> ENTERING_MAINTENANCE, an end time, a certificate serial ID and the current
> software version as initial version, next to a VERSION file. It then makes
> one write fail in three ways: the real
> {{SetNodeOperationalStateCommandHandler}} with an interrupted thread, the
> same handler with an I/O error when the temporary file is created, and
> {{YamlUtils.dump}} called directly with a {{Yaml}} whose writer fails every
> write, as a full disk does. After each failure the file has length 0, and
> reading it with {{ContainerUtils.readDatanodeDetailsFrom}}, as a restart
> does, gives the right UUID with {{persistedOpState=IN_SERVICE}}, expiry 0,
> null certificate serial ID and {{initialVersion=DEFAULT_VERSION}}. A passing
> test means the defect is present.
> h3. Patch
> [^CR-6-replace-datanode-id-atomically.patch], against
> b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. It also applies to master at
> 20d95ed37fe.
> {{writeDatanodeDetailsTo}} no longer deletes and recreates an existing file;
> it only creates the parent directory when the file does not exist yet.
> {{YamlUtils.dump}} now flushes and closes the stream only after {{yaml.dump}}
> succeeded, which moves the temporary file over the target, and calls
> {{abort()}} on any failure, which deletes the temporary file and leaves the
> target alone. A failed write now keeps the previous content and leaves no
> temporary file. A successful write produces the same file as before. The
> change is in the shared {{YamlUtils.dump}}, so its other callers get the same
> behaviour.
> Covered by the new {{TestYamlUtils}} ({{hadoop-hdds/framework}}): a write
> error inside {{yaml.dump}} must keep the previous content and leave no
> temporary file, and a later write must succeed; a second case covers an
> {{Error}} during serialization. Also by
> {{testFailedDatanodeIdWriteKeepsPreviousFile}} in the existing
> {{TestContainerUtils}}, for an error when the temporary file is created. Both
> fail without the change because the file is empty. With the change the tests
> of {{YamlUtils}}, {{ContainerUtils}}, {{DatanodeIdYaml}} and the other
> {{YamlUtils.dump}} callers, {{TestDecommissionAndMaintenance}} on a mini
> cluster (9 tests) and checkstyle pass. With the patch all three reproduction
> tests fail because the file keeps its 346 bytes and the old state. Under a
> file size limit of 0, a real {{File too large}} write error left the file at
> 0 bytes before the change and kept its 35 bytes after it.
> Not changed: the parent directory is not synced after the rename, so after a
> power loss some file systems may show the old file, never an empty one. A
> write error inside {{yaml.dump}} surfaces as an unchecked {{YAMLException}}
> that gets past the callers' {{catch (IOException)}}, so their handling of a
> failed write, such as the shutdown in {{InitDatanodeState}}, is skipped.
> Found by code review of the SCM datanode decommission and maintenance admin
> paths, as part of the TLA+ verification effort under HDDS-15926, on commit
> b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing. Related but not
> duplicates: HDDS-13955 (recovers the UUID from an empty file, the symptom),
> HDDS-5309 (empty {{datanode.id}} after a restart, closed without a cause),
> HDDS-12843 (empty {{datanode.id}} in {{TestDecommissionAndMaintenance}},
> fixed the in memory update order) and HDDS-12608 (VERSION file creation
> race). The attached patch is a proposal for review. Generated with Specula
> (Claude Opus 5.5 and Claude Opus 5).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
