[
https://issues.apache.org/jira/browse/HDDS-17281?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17281:
------------------------------
Attachment: TestBugCR3ReregisterDiskCheckCleanup.java
> Datanode re-registration deletes the file of a running volume health check,
> which can mark a healthy volume failed until restart
> --------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17281
> URL: https://issues.apache.org/jira/browse/HDDS-17281
> Project: Apache Ozone
> Issue Type: Bug
> Affects Versions: 1.4.0, 2.0.0, 1.4.1, 2.1.0, 2.2.0, 2.1.1, 2.1.2, 2.2.1
> Reporter: Siyao Meng
> Priority: Minor
> Attachments:
> CR-3-clean-disk-check-dir-only-on-first-tmp-dir-creation.patch,
> TestBugCR3ReregisterDiskCheckCleanup.java
>
>
> h3. Mechanism
> On every GETVERSION of an SCM endpoint,
> {{VersionEndpointTask.checkVolumeSet}} runs {{StorageVolumeUtil.checkVolume}}
> on all live volumes. It calls {{StorageVolume.createTmpDirs}}, which calls
> {{cleanTmpDiskCheckDir}} and deletes every file in {{tmp/disk-check}}.
> {{StorageVolume.check}} writes a test file into that directory, syncs it,
> reads it back and deletes it while holding the volume monitor, but the
> cleanup does not take that monitor, and the volume set write lock held by
> {{checkVolumeSet}} does not exclude the volume checker. If the cleanup runs
> while a check's file is on disk, the check fails to read or delete its file
> and counts an IO failure. With the default
> {{hdds.datanode.disk.check.io.failures.tolerated}} of 1, a second such
> failure within {{hdds.datanode.disk.check.sliding.window.timeout}} (default
> 70 min) makes the check return FAILED, {{MutableVolumeSet}} fails the volume,
> and a failed volume is not checked again until the datanode restarts.
> h3. Trigger
> An SCM endpoint goes back to GETVERSION while the volume checker is running a
> check on a volume:
> * SCM sends a reregister command when it receives a heartbeat from a datanode
> it does not know, so every SCM restart (each SCM of an HA group) makes all
> datanodes re-register with it.
> * An SCM endpoint rebuilt after an SCM DNS change (with
> {{ozone.client.failover.resolve-needed}} enabled, which is off by default),
> or added by {{ozone.scm.nodes}} reconfiguration, also starts in GETVERSION.
> Recon reregister commands do not trigger it, because the volume checks are
> skipped for the Recon endpoint. The window is the few milliseconds a check's
> file exists, and checks run every 60 min by default (on demand at most every
> 10 min), so a collision is very unlikely per re-registration and two are
> needed with the default tolerance (one with a tolerance of 0).
> h3. Impact
> * A healthy volume can be marked failed and stay failed until restart; its
> containers are then handled as lost by SCM.
> * If every volume fails this way the datanode shuts down (argued from the
> code, not run).
> h3. Reproduction
> Deterministic, on unmodified source:
> [^TestBugCR3ReregisterDiskCheckCleanup.java]
> ({{hadoop-hdds/container-service}}) uses a real {{MutableVolumeSet}} with one
> data volume, a real {{StorageVolumeChecker}} and a real
> {{VersionEndpointTask}} against a mocked SCM. Through the existing
> {{DiskCheckUtil.setTestImpl}} hook the read and write check writes its test
> file, lets one re-registration run while the file is on disk, and then
> deletes the file as the real check does. After two {{checkAllVolumes}} runs
> with the default tolerance (only the minimum gap between checks is set to 0)
> the test asserts that no volume failed. On unmodified code the volume is in
> the failed volume list; with the patch it stays healthy.
> h3. Patch
> [^CR-3-clean-disk-check-dir-only-on-first-tmp-dir-creation.patch], against
> b10a7d427bdf83807a7e48a1be507d3b02d5a503. It also applies to master at
> e0de543cbe9.
> {{StorageVolume.createTmpDirs}} empties the disk check directory only on its
> first call for a volume, which in production is the first GETVERSION after
> the datanode starts, so files left by a previous process are still removed at
> startup and by {{shutdown}}. Later calls still create the directories but no
> longer delete files that can only belong to a running check. Taking the
> volume monitor in the cleanup was not chosen, because a check can block on a
> bad disk and GETVERSION would then wait for it while holding the volume set
> write lock.
> Covered by {{testCreateTmpDirsAgainKeepsDiskCheckFileInUse}} in the existing
> {{TestHddsVolume}}; the existing {{testClearVolumeHealthCheckDir}} still
> covers the first call cleanup. Without the change the new test fails because
> the file is gone; with it the volume, volume checker,
> {{TestKeyValueHandler}}, {{TestContainerReader}},
> {{TestRunningDatanodeState}} and {{TestEndPoint}} suites pass, checkstyle is
> clean.
> {{HddsVolume.createTmpDirs}} also empties {{tmp/deleted-containers}} on every
> re-registration, which could make a concurrent container delete fail (argued,
> not run, not changed by this patch).
> Found by code review of the datanode registration and heartbeat paths, as
> part of the TLA+ verification effort under HDDS-15926, on commit
> b10a7d427bdf83807a7e48a1be507d3b02d5a503. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing. Related but not
> duplicates: HDDS-8782 (added the read and write volume check and the cleanup)
> and HDDS-17013 (import leftovers in the volume tmp directory). The attached
> patch is a proposal for review. Generated with Specula (Claude Opus 5.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]
