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