[ 
https://issues.apache.org/jira/browse/HDDS-17619?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng updated HDDS-17619:
------------------------------
    Affects Version/s: 2.2.1
                       2.0.0
                       1.4.0
                       2.1.2
                       2.1.0
                       1.4.1
                       2.1.1
                       2.2.2
                       2.2.0

> EC under replication handler runs over replication inline while a 
> reconstruction is pending and can delete one of its sources
> -----------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HDDS-17619
>                 URL: https://issues.apache.org/jira/browse/HDDS-17619
>             Project: Apache Ozone
>          Issue Type: Bug
>    Affects Versions: 1.4.0, 2.0.0, 1.4.1, 2.1.0, 2.2.0, 2.1.1, 2.1.2, 2.2.1, 
> 2.2.2
>            Reporter: Siyao Meng
>            Priority: Minor
>         Attachments: 
> MC-5-no-inline-ec-over-replication-while-add-pending.patch, 
> TestBugMC5InlineDeleteOfReconstructionSource.java
>
>
> h3. Mechanism
> When {{ECUnderReplicationHandler.processAndSendCommands}} gets 
> {{FAILED_TO_FIND_SUITABLE_NODE}} at one of its stages and the container is 
> also over replicated, it calls 
> {{ReplicationManager.processOverReplicatedContainer}}, so that a delete may 
> free a node for the next run. It does this even when a reconstruction or 
> replication of the same container is pending, whether it was sent earlier in 
> the same run or in an earlier run. {{ECOverReplicationHandler}} drops only 
> replicas with a pending delete from its candidates and picks the copy to 
> delete by rack crowding 
> ({{SCMCommonPlacementPolicy.replicasToRemoveToFixOverreplication}}). It does 
> not look at the pending ADD ops, although each one carries the command with 
> its sources. So when the copy a pending reconstruction reads from sits on the 
> more crowded rack, SCM deletes it.
> HDDS-7092 avoided this: it handed off to the over replication handler only 
> when no command had been created in the run, because such commands "may 
> attempt to use sources the over-rep handler would remove". HDDS-8333 (partial 
> EC reconstruction) dropped that check on purpose, so that over replication 
> can free a node for the next run after a partial success on a small cluster, 
> but did not address the source concern; 
> {{testUnderRepWithDecommissionAndNotEnoughNodes}} was changed to expect the 
> call while its comment still describes the old behaviour. Commands from 
> earlier runs were never considered. The patch below reverses the HDDS-8333 
> choice only while an add is pending.
> h3. Trigger
> Example with rs-3-2: index 1 has two copies (for example after a node 
> returned with an old copy), indexes 2 and 3 one each, indexes 4 and 5 are 
> lost, and only one node can take a new replica.
> # Run 1: the placement policy finds one target, so a partial reconstruction 
> of index 4 is sent, reading from one of the index 1 copies, index 2 and index 
> 3. The container is requeued ({{InsufficientDatanodesException}}).
> # Run 2, about {{hdds.scm.replication.under.replicated.interval}} (30s) 
> later: no target for index 5, so the over replication handler runs inline and 
> deletes one copy of index 1. If that is the reconstruction's source and the 
> reconstruction has not finished yet, it fails on the datanode, since it needs 
> all three listed sources.
> The same happens within one run when a reconstruction is sent for a missing 
> index and the decommission or maintenance stage then finds no target.
> h3. Impact
> * The reconstruction is wasted. SCM is not told about the failure 
> (HDDS-15327), so it keeps the pending add and does not send a new 
> reconstruction until {{hdds.scm.replication.event.timeout}} (12 minutes) 
> expires.
> * In the example the container was already at zero redundancy; the bug 
> extends that from one reconstruction to the event timeout plus a second 
> reconstruction. A further loss in that window loses data.
> * It needs a duplicate index, a target shortfall and a pending add at the 
> same time, so it is not common. Small clusters and clusters short of space or 
> with overloaded nodes are the likely places.
> The datanode failure is argued from {{ECReconstructionCoordinator}} 
> ({{getBlockDataMap}} lists blocks on every source, and with two indexes lost 
> on rs-3-2 there is no spare source), not run on a real datanode.
> h3. Reproduction
> Deterministic unit test with the real {{ECUnderReplicationHandler}} and 
> {{ECOverReplicationHandler}}: 
> [^TestBugMC5InlineDeleteOfReconstructionSource.java] 
> ({{hadoop-hdds/server-scm}}). {{ReplicationManager}} is mocked to record 
> commands and to route {{processOverReplicatedContainer}} to the real over 
> replication handler; the placement policy offers one free node. After run 1 
> the test puts the index 1 copy the reconstruction reads from on the crowded 
> rack, then asserts that run 2 deletes no source. On unmodified source it 
> fails as below; which index 1 copy (n1 or n1b) becomes the source depends on 
> random node IDs, and the deleted copy is always that source:
> {noformat}
> Run 1: reconstruct index 4 on n4, sources [n1 (index 1), n2 (index 2), n3 
> (index 3)]
> Run 2: delete commands to [n1 (index 1)]
> [replicas deleted while a reconstruction is pending] Expecting ["n1 (index 
> 1)"] not to contain [...] but found ["n1 (index 1)"]
> {noformat}
> With the patch it passes and run 2 sends no delete.
> h3. Suggested fix
> Skip the inline over replication while the container has a pending add, as in 
> the patch. A more precise alternative is to drop the sources of pending 
> reconstruction and replication commands (reachable through 
> {{ContainerReplicaOp.getCommand}}) from the delete candidates in 
> {{ECOverReplicationHandler}}. It is larger, and for the duplicated index it 
> ends the same way, because the existing check refuses to delete the last 
> remaining candidate of an index.
> h3. Patch
> [^MC-5-no-inline-ec-over-replication-while-add-pending.patch], against 
> e4ae41fc4047c99af983020b4f33bd297c9d0ac5. It also applies to master at 
> 3fc2387040f.
> {{ECUnderReplicationHandler}} runs the inline over replication only if 
> {{ECContainerReplicaCount.hasPendingAdd()}} (new accessor) is false. The 
> replica count holds the pending ops of earlier runs and every command sent in 
> the current run, so both variants are covered. The inline delete then runs on 
> the first run in which no add is pending, typically after the reconstruction 
> completes, so the small cluster case it exists for still progresses, one 
> reconstruction later than before. While a reconstruction keeps failing and 
> being resent after each expiry, and a later stage keeps finding no target, 
> the inline delete keeps waiting, where before it ran on every pass; that 
> delays the over replication fix, not redundancy. Removal of UNHEALTHY 
> replicas on the same path is unchanged.
> New test {{testOverRepNotHandledWhileReconstructionPending}} and the 
> corrected {{testUnderRepWithDecommissionAndNotEnoughNodes}} (now {{never()}}, 
> matching its comment) in {{TestECUnderReplicationHandler}} fail without the 
> change with {{NeverWantedButInvoked}} on {{processOverReplicatedContainer}}. 
> With it, the EC handler, replica count, {{ReplicationManager}}, scenario and 
> under replicated processor suites pass (215 tests) and checkstyle is clean. 
> No mini cluster test was run.
> Found by TLA+ model checking and code review of the SCM EC container 
> replication and placement repair paths under HDDS-15926, on commit 
> e4ae41fc4047c99af983020b4f33bd297c9d0ac5. Checked against HDDS issues and 
> apache/ozone pull requests for duplicates before filing; the nearest is 
> HDDS-7948, closed as Not A Problem for an occasional source deleted under a 
> reconstruction, which did not look at this path. The attached patch is a 
> proposal for review. Generated with Specula (Claude Opus 5.5).



--
This message was sent by Atlassian Jira
(v10.3.26#10030026)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to