[ 
https://issues.apache.org/jira/browse/HDDS-17281?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng updated HDDS-17281:
------------------------------
    Affects Version/s: 2.2.1
                       2.0.0
                       1.4.0
                       2.1.2
                       2.1.0
                       1.4.1
                       2.1.1
                       2.2.0

> 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]

Reply via email to