Siyao Meng created HDDS-16886:
---------------------------------
Summary: Container move is reported COMPLETED when
ReplicationManager deletes the source replica before the target replica exists
Key: HDDS-16886
URL: https://issues.apache.org/jira/browse/HDDS-16886
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments: MC-3-finish-move-only-on-its-own-source-delete.patch,
TestBugMC3SourceDeleteCompletesMove.java
h3. Mechanism
{{MoveManager.notifyContainerOpCompleted}} attributes a completed pending op to
a move only by its type and datanode. Any completed DELETE of the source
replica calls {{completeMove(COMPLETED)}} and removes the move from
{{pendingMoves}}, also when the move is still waiting for its ADD on the target
and has not sent a delete itself. {{ContainerReplicaPendingOps}} notifies every
subscriber of every completed op, including deletes that ReplicationManager
sends on its own, and nothing stops ReplicationManager from deleting the source
of a pending move. {{notifyContainerOpExpired}} has the same match, so an
expired ReplicationManager DELETE of the source ends the move as
{{DELETION_FAIL_TIME_OUT}}.
The comment in {{MoveManager.handleSuccessfulAdd}} ("if the target is present
but source disappears somehow, we can consider move is successful") shows that
COMPLETED is meant to require a replica on the target.
h3. Trigger
# The container balancer starts a move of a CLOSED container (reproduced for
Ratis); the ADD on the target is pending.
# Before the ADD completes, ReplicationManager deletes the source replica:
#* {{RatisOverReplicationHandler}} picks the source as the excess replica. With
default config this needs the container to gain a replica during the move (for
example a datanode that was dead comes back). With
{{hdds.container.balancer.include.non.standard.containers=true}} the balancer
moves containers that are already over replicated.
#* {{EmptyContainerHandler}} deletes all replicas of a CLOSED container whose
replicas report no blocks. The balancer selects such a container only while its
used bytes are above 0 (for example leftover chunk data), and
{{MoveManager.move}} accepts it.
# The source datanode reports the deletion and the move completes as COMPLETED.
h3. Impact
* {{ContainerBalancerTask}} treats the move as successful: the completed move
count and the moved bytes (the container's used bytes) are increased, and no
failure is recorded. These values are shown by {{ozone admin containerbalancer
status -v}} and in the balancer metrics.
* The result is final. The move is no longer tracked, so the later failure or
expiry of the target ADD is never reported.
* No data loss: the container keeps its replication factor (ReplicationManager
only deleted an excess replica), and an empty container is being deleted
anyway. No scheduling decision reads the wrong result.
h3. Reproduction
PASS, deterministic, unmodified source.
[^TestBugMC3SourceDeleteCompletesMove.java] ({{hadoop-hdds/server-scm}}) wires
a real {{ReplicationManager}}, {{ContainerReplicaPendingOps}} and
{{MoveManager}} as SCM does, with mocked container store, node manager and
placement policy and no datanodes. Datanode reports are simulated with
{{ContainerReplicaPendingOps.completeDeleteReplica}}, and the over replication
queue is drained once with {{OverReplicatedProcessor}}. Three cases: a HEALTHY
container that gains a fourth replica during the move (default config), an over
replicated container with {{include.non.standard.containers}}, and a CLOSED
container whose replicas report no blocks but which still reports 100 used
bytes. In each the move is COMPLETED while the target has no replica and its
ADD is still pending, and it stays COMPLETED after the ADD expires. A passing
test means the defect is present. The balancer task itself was not run; its
handling of COMPLETED is read from {{ContainerBalancerTask.moveContainer}}.
h3. Patch
[^MC-3-finish-move-only-on-its-own-source-delete.patch], against
b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. It also applies to master at
6818707698c.
{{MoveOperation}} records when {{handleSuccessfulAdd}} sends the move's own
DELETE, and a source DELETE completes or expires the move only after that. An
earlier DELETE of the source leaves the result to the target ADD: if it
completes, the existing missing source branch reports COMPLETED; if it expires,
the move ends as {{REPLICATION_FAIL_TIME_OUT}}. Moves in which only the move
deletes the source behave as before. For an affected move the balancer now
waits for the ADD instead of finishing at once, up to
{{hdds.container.balancer.move.replication.timeout}} (default 50m).
Covered by {{testSourceDeletedBeforeReplicationDoesNotCompleteMove}} and
{{testSourceDeleteExpiredBeforeReplicationDoesNotFailMove}} in the existing
{{TestMoveManager}}. Without the change they fail with the move already
COMPLETED and DELETION_FAIL_TIME_OUT. With it {{TestMoveManager}}, the
{{TestContainerBalancer*}} suites, {{TestContainerReplicaPendingOps}} and
{{TestReplicationManager}} pass (517 tests), checkstyle is clean, and the three
reproduction tests fail because the move is still pending after the source
delete.
Found by TLA+ model checking and code review of the SCM container move and
balancer paths under HDDS-15926, on commit
b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. 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]
