[
https://issues.apache.org/jira/browse/HDDS-16829?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16829:
------------------------------
Attachment:
CR-10-finalize-block-for-missing-block-should-not-fail-container.patch
> FinalizeBlock for a block the datanode does not have marks the container
> UNHEALTHY and closes the pipeline
> ----------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16829
> URL: https://issues.apache.org/jira/browse/HDDS-16829
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments:
> CR-10-finalize-block-for-missing-block-should-not-fail-container.patch
>
>
> h3. Mechanism
> {{KeyValueHandler.handleFinalizeBlock}} reads the block with
> {{blockManager.getBlock}} before it writes anything. If the datanode does not
> have the block, that throws {{NO_SUCH_BLOCK}}, or {{CONTAINER_NOT_FOUND}} if
> it does not have the container. Nothing was changed at that point.
> The result is nevertheless handled as a failed write, in two places:
> * {{HddsDispatcher.dispatchRequest}}: neither result is in
> {{canIgnoreException}}, so the container replica is marked {{UNHEALTHY}} and
> a close action is sent to SCM.
> * {{ContainerStateMachine.applyTransaction}}: neither result is in its list
> of tolerated results, so the state machine is marked unhealthy, the container
> is added to {{unhealthyContainers}} and
> {{XceiverServerRatis.handleApplyTransactionFailure}} asks SCM to close the
> pipeline.
> FinalizeBlock is a Ratis log entry, so every replica applies it and does the
> same.
> h3. Trigger
> A FinalizeBlock request for a block that was allocated but never written (no
> PutBlock reached the datanodes) is applied.
> h3. Impact
> The container is marked {{UNHEALTHY}} on every datanode of the pipeline and
> the pipeline is closed, although the request did not change anything and the
> replicas do not differ. The caller gets the same error as it should,
> {{NO_SUCH_BLOCK}} or {{CONTAINER_NOT_FOUND}}.
> The datanode and SCM log lines of this are in the description of HDDS-11550
> ("Operation: FinalizeBlock ... Result: NO_SUCH_BLOCK", then "reported
> UNHEALTHY replica" and "Received pipeline action CLOSE"). That issue is about
> the error the caller gets. This one is about what the datanode does with the
> container and the pipeline.
> h3. Reproduction
> There is no standalone reproduction for this issue. The regression tests in
> the attached patch cover it. On unmodified source
> {{TestHddsDispatcher.testFinalizeBlockForMissingBlockDoesNotMarkContainerUnhealthy}}
> fails because the container is no longer open after the request, and
> {{testApplyFinalizeBlockForMissingBlockDoesNotFailStateMachine}} fails for
> both results, on leader and follower, with a {{StorageContainerException}}
> from {{applyTransaction}}.
> h3. Patch
> [^CR-10-finalize-block-for-missing-block-should-not-fail-container.patch],
> against ea69b4a7d9abd040e5259dbcbb7c6ce52eb5d199. It also applies to master
> at bdff9801bb2.
> A new {{ContainerUtils.isFinalizeOfMissingBlock(cmdType, result)}} is true
> only for a FinalizeBlock with {{NO_SUCH_BLOCK}} or {{CONTAINER_NOT_FOUND}}.
> Both places above skip their failure handling when it is true. The reply to
> the caller is unchanged. Any other failure of FinalizeBlock, and the same two
> results for any other command, are handled as before (both are covered as the
> negative case of the two tests). This is the same kind of change as
> HDDS-15301 ({{MALFORMED_REQUEST}}) and HDDS-14831
> ({{CONTAINER_ALREADY_EXISTS}}). A predicate on the command type is used
> because {{canIgnoreException}} only sees the result, and the two results stay
> failures for every other command.
> {{ContainerStateMachine.handleCommandResult}} has a similar check and is
> deliberately left alone: it handles the result of the write of chunk data
> only, and FinalizeBlock does not pass through it.
> Compatibility: no new request, field or persisted state. A datanode without
> the change still marks its replica and asks for the pipeline to be closed
> when it applies such an entry, as all of them do today. No layout feature is
> used.
> Tests: the two named above, in the existing {{TestHddsDispatcher}} and
> {{ContainerStateMachineTests}} (run as {{TestContainerStateMachineLeader}}
> and {{TestContainerStateMachineFollower}}). With the patch these three
> classes pass (38 tests), as does {{TestFinalizeBlock}} on a mini cluster (2),
> and checkstyle is clean.
> The patch on HDDS-16828 extends the same condition in
> {{ContainerStateMachine.applyTransaction}}, so the later of the two to land
> needs a one line rebase.
> Found by code review of the lease recovery paths for hsynced files, as part
> of the TLA+ verification effort under HDDS-15926, on commit
> ea69b4a7d9abd040e5259dbcbb7c6ce52eb5d199. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing. Related but not
> duplicates: HDDS-11550 (open, see above), HDDS-10242 (resolved, changed how
> the caller handles the error), HDDS-6549 (open, the general question whether
> one failed write should mark a replica unhealthy). 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]
