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]