[jira] [Updated] (HDDS-17227) A failed rewrite of datanode.id leaves it empty, so the datanode loses its operational state and initial version on the next start

["Siyao Meng (Jira)" <ji...@apache.org>]

     [ 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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org