Siyao Meng created HDDS-17017:
---------------------------------

             Summary: Per disk RocksDB of a failed datanode volume is reopened 
by a racing write or compaction and never closed until restart
                 Key: HDDS-17017
                 URL: https://issues.apache.org/jira/browse/HDDS-17017
             Project: Apache Ozone
          Issue Type: Bug
            Reporter: Siyao Meng
         Attachments: MC-1-do-not-reopen-db-of-failed-volume.patch, 
TestBugMC1FailedVolumeDbReopen.java

h3. Mechanism
{{HddsVolume.failVolume}} marks the volume FAILED and then {{closeDbStore}} 
removes its schema V3 per disk RocksDB from {{DatanodeStoreCache}}, stops it 
and sets {{dbLoaded}} to false. {{DatanodeStoreCache.getDB}} opens any DB path 
that is not cached and does not look at the volume, and neither of its callers 
checks it: {{BlockUtils.getDB}} (PutBlock, reads such as GetBlock and 
ListBlock, block deletion, close, scan, import) and {{HddsVolume.compactDb}}. A 
store reopened this way is never closed again, because {{closeDbStore}} returns 
early once {{dbLoaded}} is false. Only {{shutdownCache}} at process exit 
releases it.

h3. Trigger
# The volume checker fails a data volume whose DB directory can still be opened 
(for example after a check timeout). {{MutableVolumeSet.handleVolumeFailures}} 
closes its DB and then, after releasing its lock, runs the listener that 
removes the volume's containers.
# Before the listener has removed a container, or while a request is already 
past its container lookup, a PutBlock for that container reaches 
{{BlockUtils.getDB}}. Or a compaction task that {{OzoneContainer.compactDb}} 
queued from a volume list taken before the failure runs 
{{HddsVolume.compactDb}} on the volume ({{compactDb}} opens the DB before 
deciding whether to compact).
# The DB is opened again and cached.

h3. Impact
* The RocksDB instance, its LOCK file and its file handles stay open on the 
failed disk until the datanode restarts. Later {{failVolume}} and {{shutdown}} 
calls on the volume do nothing.
* The PutBlock in the window is acknowledged with SUCCESS although the volume 
has failed.
* The window is short and needs a volume failure; no client action can cause 
one.

h3. Reproduction
PASS, deterministic, unmodified source. [^TestBugMC1FailedVolumeDbReopen.java] 
({{hadoop-hdds/container-service}}) uses two schema V3 data volumes and a 
volume checker that fails one of them, so the real {{handleVolumeFailures}} 
closes its DB. The first test sends WriteChunk and PutBlock through 
{{HddsDispatcher.dispatch}} before the real container sweep runs (the only 
timing aid is a listener that waits for the request); both return SUCCESS, the 
DB is cached again with its RocksDB LOCK held, and it stays so after 
{{HddsVolume.failVolume}} and {{shutdown}}. The second test runs 
{{HddsVolume.compactDb}} on the failed volume, as a queued compaction task 
does, with the same result. A passing test means the defect is present.

h3. Patch
[^MC-1-do-not-reopen-db-of-failed-volume.patch], against 
cce1ca28944886218132f01c0e271c1fd0700078. It also applies to master at 
2536fe9cbc3.

{{DatanodeStoreCache.getDB}} gets an overload that takes the volume and refuses 
to open the DB when the volume or its DB volume has failed. The check is inside 
the existing synchronized block, after the cache miss, and {{removeDB}} now 
removes the entry under the same monitor. {{HddsVolume.failVolume}} and 
{{DbVolume.failVolume}} both set the FAILED state before they remove the DB, so 
a {{getDB}} that misses because of that removal always sees the failure, and 
one that opened the DB before it is closed by it; a check in the callers would 
still race. The DB volume case matters because {{DbVolume.failVolume}} removes 
the DBs of the data volumes on it while those stay NORMAL. {{BlockUtils.getDB}} 
and {{HddsVolume.compactDb}} pass the volume. Opening on a miss stays as it is 
for healthy volumes and for callers without a volume.

Covered by {{testDbStoreNotReopenedOnFailedVolume}} in the existing 
{{TestHddsVolume}}, for a failed data volume and for a failed DB volume. 
Without the change both cases fail because {{compactDb}} puts the DB back in 
the cache. With it {{TestHddsVolume}} and the neighbouring volume, DB cache, 
dispatcher, handler, block manager and container suites (422 tests) pass, 
{{TestDatanodeHddsVolumeFailureDetection}} and 
{{TestDatanodeHddsVolumeFailureToleration}} on a mini cluster pass (run with 
the related volume failure patches applied together), and checkstyle is clean. 
With the patch the compaction test of the reproduction fails because the DB is 
not reopened, and in the PutBlock test the PutBlock fails with "Error opening 
DB".

After the patch such a PutBlock takes the existing failure path of 
{{HddsDispatcher.dispatch}} for a write to an open container on a failed 
volume, as a WriteChunk that hits an IO error on the dead disk does today: over 
Ratis the client gets a {{StateMachineException}} and retries on another 
pipeline, over gRPC (EC) the stream fails and the writer excludes the datanode 
(from the code, not run). Reads get {{UNABLE_TO_READ_METADATA_DB}}. Skipping 
the dispatcher's UNHEALTHY precondition for containers on a failed volume, so 
that the write gets a result code instead, would be a separate follow up. 
WriteChunk does not open the DB and is not changed.

With the fix for the related issue on DB volume failures applied as well, the 
data volumes on a failed DB volume are then failed by the listener and their 
DBs closed; this check covers the time in between. The two patches touch 
different files and apply together.

Found by TLA+ model checking and code review of the datanode volume and 
per-disk RocksDB lifecycle paths under HDDS-15926, on commit 
cce1ca28944886218132f01c0e271c1fd0700078. Checked against HDDS issues and 
apache/ozone pull requests for duplicates before filing. Related but not 
duplicates: HDDS-16982 (a test helper reopens a stopped datanode's DB through 
the same path) and HDDS-13994 (reopen a DB closed on a full disk, which would 
need to keep this check). 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