[
https://issues.apache.org/jira/browse/HDDS-16885?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Chi-Hsuan Huang updated HDDS-16885:
-----------------------------------
Fix Version/s: 2.3.0
> 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
> Assignee: Andy Huang
> Priority: Minor
> Labels: pull-request-available
> Fix For: 2.3.0
>
> 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]
