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