Siyao Meng created HDDS-16885:
---------------------------------
Summary: TestMoveManager negative checks verify the wrong
sendDeleteCommand overload and cannot fail
Key: HDDS-16885
URL: https://issues.apache.org/jira/browse/HDDS-16885
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments:
CR-5-verify-deadline-overload-in-move-manager-negative-checks.patch,
TestBugCR5DeleteVerifyWrongOverload.java
h3. Mechanism
{{testMoveCompleteSrcNoLongerPresent}}, {{testMoveCompleteSrcNotHealthy}},
{{testMoveCompleteSrcNotInService}} and
{{testMoveCompleteFutureReplicasUnhealthy}} in {{TestMoveManager}} assert that
no source DELETE is sent with {{verify(replicationManager,
times(0)).sendDeleteCommand(eq(containerInfo), eq(0), eq(src), eq(true))}}.
That is the 4 argument overload of {{ReplicationManager.sendDeleteCommand}}.
Since HDDS-8168 {{MoveManager}} only calls the overload that takes a deadline
(now 5 arguments), and on the Mockito mock the overloads are separate methods,
so these checks pass whether or not a DELETE is sent. HDDS-8168 and HDDS-8230
updated the positive checks in the same file but not these four.
h3. Trigger
Any change that makes the source deletion safety branches of
{{MoveManager.handleSuccessfulAdd}} (source missing, source not in service,
source unhealthy, placement would break) send the source DELETE.
h3. Impact
Test only. The production code is correct today: with the checks corrected, all
four tests pass. The four tests cannot detect a regression that deletes the
source replica on those branches.
h3. Reproduction
PASS, deterministic, unmodified source.
[^TestBugCR5DeleteVerifyWrongOverload.java] ({{hadoop-hdds/server-scm}}) uses
the same mocks as {{TestMoveManager}}, drives a successful move in which the
source DELETE is sent, and shows that the 4 argument {{times(0)}} check still
passes while the same check on the 5 argument overload fails. In addition, with
{{MoveManager}} changed locally to send a source DELETE at the start of
{{handleSuccessfulAdd}}, the original four tests pass and the corrected ones
fail (not part of the attachments).
h3. Patch
[^CR-5-verify-deadline-overload-in-move-manager-negative-checks.patch], against
b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. It also applies to master at
6818707698c.
The four checks verify {{sendDeleteCommand(eq(containerInfo), eq(0), eq(src),
eq(true), anyLong())}}, as the positive checks in the file do. Test change
only. {{TestMoveManager}} passes (25 tests) and checkstyle is clean.
Found by code review of the SCM container move and balancer paths, as part of
the TLA+ verification effort 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]
