[
https://issues.apache.org/jira/browse/HDDS-17017?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17017:
------------------------------
Attachment: TestBugMC1FailedVolumeDbReopen.java
> 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
> Priority: Major
> 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]
