[
https://issues.apache.org/jira/browse/HDDS-17619?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17619:
------------------------------
Attachment: MC-5-no-inline-ec-over-replication-while-add-pending.patch
> 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]
