Siyao Meng created HDDS-16829:
---------------------------------
Summary: 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
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]
