[
https://issues.apache.org/jira/browse/HDDS-17018?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17018:
------------------------------
Attachment: MC-2-reject-container-on-failed-volume.patch
> Container imported or created while its datanode volume fails stays
> registered on the failed volume and is reported to SCM as live
> ----------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17018
> URL: https://issues.apache.org/jira/browse/HDDS-17018
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments: MC-2-reject-container-on-failed-volume.patch,
> TestBugMC2ContainerOnFailedVolume.java
>
>
> h3. Mechanism
> When a data volume fails, {{MutableVolumeSet.handleVolumeFailures}} marks it
> FAILED and, after releasing its lock, the failed volume listener
> {{ContainerSet.handleVolumeFailures}} removes the volume's containers in a
> single pass and sends a full container report. Neither the push import path
> ({{ContainerImporter.importContainer}}) nor the create path
> ({{KeyValueHandler.handleCreateContainer}}) checks the volume state before
> {{ContainerSet.addContainer}}. A container added after that pass stays in the
> container set on the FAILED volume and is never removed.
> h3. Trigger
> # A push replication of a container reaches the datanode and the target
> volume is chosen on the first chunk, or a CreateContainer is processed and
> {{KeyValueContainer.create}} has picked its volume.
> # Before the container is added to the set, the volume checker fails that
> volume and the listener sweep runs.
> # The import or create then adds the container.
> No timing aid is needed for the import path when the volume fails while the
> tarball is still streaming. That holds while the per disk DB of a failed
> volume can be reopened (HDDS-17017): with that fixed, an import whose volume
> fails while streaming is refused when it loads the container metadata into
> the DB, and the window left for import is between that load and
> {{addContainer}}. The create route keeps its window between
> {{KeyValueContainer.create}} and {{addContainer}}; the create test of the
> reproduction still passes with only that other fix applied.
> h3. Impact
> * The datanode keeps the container on a volume it has declared failed and
> includes it in its container reports, so SCM counts it as a live replica.
> * It stays until the datanode restarts. Delete (exercised in the
> reproduction) and unhealthy handling (argued from the code) skip containers
> on failed volumes.
> h3. Reproduction
> PASS, deterministic, unmodified source.
> [^TestBugMC2ContainerOnFailedVolume.java] ({{hadoop-hdds/container-service}})
> uses two schema V3 data volumes, the real {{handleVolumeFailures}} and the
> real container sweep. The first test streams a push import through
> {{SendContainerRequestHandler}} and fails the target volume between the first
> and the last chunk: the imported container is in the set and in the container
> report, while the container that was on the same volume before the failure
> was swept. The second test sends a CreateContainer through the handler and
> fails the volume while a spy holds {{addContainer}}: the create returns
> SUCCESS and the container is in the set and in the report. A passing test
> means the defect is present.
> h3. Patch
> [^MC-2-reject-container-on-failed-volume.patch], against
> cce1ca28944886218132f01c0e271c1fd0700078. It also applies to master at
> 2536fe9cbc3.
> {{ContainerSet.addContainer}} checks the volume state right after the
> container is put into the map. If the volume has failed it removes that entry
> again and throws {{StorageContainerException}} with
> {{CONTAINER_INTERNAL_ERROR}}, before the container is counted on the volume.
> The check has to come after the put: the volume is marked FAILED before the
> sweep, so either the sweep sees the container or this check sees the failure.
> One check in the shared method covers import, create and
> {{addContainerByOverwriteMissingContainer}}, and their callers already handle
> an exception from it. For a rejected create {{KeyValueHandler}} returns
> {{CONTAINER_INTERNAL_ERROR}}; an explicit CreateContainer sent through
> {{HddsDispatcher.dispatch}} then ends in the dispatcher's existing
> {{NullPointerException}} for a failed create, as for any other failed
> explicit create (from the code, not run).
> Covered by {{testAddContainerOnFailedVolumeIsRejected}} in the existing
> {{TestContainerSet}}. Without the change it fails because the container on
> the failed volume is added. {{TestKeyValueHandler.testDeleteContainer}} added
> a container after failing its volume and now adds it first. With the change
> the container set, handler, dispatcher, import, container reader, disk
> balancer and replication suites pass (one replication metrics test also fails
> intermittently without the change),
> {{TestDatanodeHddsVolumeFailureDetection}} and
> {{TestDatanodeHddsVolumeFailureToleration}} on a mini cluster pass, and
> checkstyle is clean. With the patch both tests of the reproduction fail
> because the container is not registered; {{KeyValueHandler}} returns
> {{CONTAINER_INTERNAL_ERROR}} for the create.
> On rejection the container directory stays on the failed disk, as for any
> other failure of {{addContainer}}.
> Not covered, left for a follow up: the disk balancer moves a container with
> {{ContainerSet.updateContainer}}, which has no volume check, and then marks
> the source replica for delete. If the destination volume fails after its
> sweep and before that swap, the moved container is registered on the failed
> volume and the healthy source copy is marked deleted (from the code, not
> run). A check there has to restore the old mapping and interacts with a
> concurrent sweep, so it is not a one line addition.
> 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. 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]
