[
https://issues.apache.org/jira/browse/HDDS-17430?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17430:
------------------------------
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.0
> After a datanode crash, every PutBlock to a container created by a Ratis data
> stream fails updateBCSID, closing the pipeline on log replay
> ------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17430
> URL: https://issues.apache.org/jira/browse/HDDS-17430
> 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
> Reporter: Siyao Meng
> Priority: Major
> Attachments:
> CR-7-re-add-stream-created-container-to-bcsid-map-on-putblock.patch,
> TestBugCR7StreamContainerMissingFromBcsidMap.java
>
>
> h3. Mechanism
> {{ContainerStateMachine.stream}} dispatches the StreamInit (or
> StreamInitWithPutBlock) request on every datanode of the pipeline. If the
> container does not exist, {{HddsDispatcher.dispatchRequest}} creates it and
> records it in the in memory {{container2BCSIDMap}}. On restart that map is
> rebuilt only from the last Ratis snapshot
> ({{ContainerStateMachine.loadSnapshot}}). {{stream}} is not called again when
> the log is replayed, applying a stream entry for a container that exists adds
> nothing to the map, and the only re-add on replay is the one HDDS-1843 added
> for WriteChunk COMMIT_DATA, which a streamed container never has. So after a
> restart from a snapshot taken before the container was created, every
> PutBlock for that container on that datanode is applied by the handler and
> then fails
> {{Preconditions.checkArgument(container2BCSIDMap.containsKey(containerId))}}
> in {{HddsDispatcher.updateBCSID}} with an IllegalArgumentException.
> h3. Trigger
> # Ratis data stream is enabled ({{hdds.container.ratis.datastream.enabled}},
> off by default) and a streaming write creates a new container.
> # A datanode of that pipeline stops without taking a Ratis snapshot (kill -9,
> OOM kill, power loss). A graceful stop takes one
> ({{raft.server.snapshot.trigger-when-stop.enabled}} defaults to true in Ratis
> 3.3.1 and Ozone does not change it) and hides the problem. Automatic
> snapshots are taken every {{hdds.ratis.snapshot.threshold}} entries (default
> 100000), so a recently created container is usually not in the last one.
> # The datanode restarts.
> h3. Impact
> * PutBlock through Raft (the default for streams): the PutBlocks of the
> stream are Raft log entries of their own, and they can come before the
> stream's entry in the log. Replaying the first one fails, the datanode's
> {{ContainerStateMachine}} is marked unhealthy and it asks SCM to close the
> pipeline. Shown in the reproduction ({{failed to applyTransaction at logIndex
> 1 for PutBlock}}, then {{pipeline Action CLOSE}}).
> * PutBlock without Raft
> ({{ozone.client.datastream.putblock.without.raft.enabled}}, master only): the
> replay succeeds, but every later stream PutBlock to that container on the
> restarted datanode (the one sent in the middle of the stream and the one at
> close) fails after it has been persisted. The client reports {{Failed to
> commit PutBlock ... through the data stream}} and retries the block; the
> aborted block stays behind, since its PutBlock was already persisted. The
> container stays out of the map until it is closed. Shown in the reproduction.
> * No key data was lost in these runs. By code reading only: while the
> container is missing from the map, later snapshots miss it too, so its BCSID
> is not validated on restart, losing it would not mark it missing, and
> {{notifyGroupRemove}} does not quasi close it.
> h3. Reproduction
> [^TestBugCR7StreamContainerMissingFromBcsidMap.java]
> ({{hadoop-ozone/integration-test}}) starts a 3 datanode mini cluster whose
> only Ratis change is
> {{hdds.ratis.raft.server.snapshot.trigger-when-stop.enabled=false}}, which
> leaves the snapshot state a kill -9 leaves. It writes a 600 byte stream key
> into a new container (small buffers, so one PutBlock is sent in the middle of
> the stream), restarts a follower without waiting for it to go stale, waits
> for its log replay and writes a second key to the same container. It runs
> once with PutBlock through Raft and once without. On the unfixed commit both
> fail: the first at {{state machine of the restarted datanode after log
> replay}}, the second at {{container2BCSIDMap of the restarted datanode after
> the second key}} (empty, read by reflection). With the patch both pass. A
> failing test means the defect is present. A real process kill was not run.
> h3. Patch
> [^CR-7-re-add-stream-created-container-to-bcsid-map-on-putblock.patch],
> against 7212465c0bb19615708c144238317db51c2a696d. It also applies to master
> at ae71e2dd50c.
> {{HddsDispatcher.dispatchRequest}} already puts an existing container that is
> missing from the map back with its current BCSID, but only for WriteChunk
> COMMIT_DATA (HDDS-1843). The patch runs that same block for PutBlock and
> PutSmallFile whenever the request carries {{container2BCSIDMap}} (Raft apply,
> stream command, stream link), which are the commands {{updateBCSID}} checks.
> The re-add now uses {{putIfAbsent}}, because stream PutBlocks do not run on
> the apply thread and a plain {{put}} could overwrite a newer BCSID. This puts
> the container back into the map on the first PutBlock to it after the
> restart: during log replay with PutBlock through Raft, at the next stream
> PutBlock without Raft. It changes nothing when the container is already in
> the map or does not exist. Re-adding only when the stream's own entry is
> applied would not be enough, because the Raft PutBlocks can precede it in the
> log. With PutBlock without Raft the log has no PutBlock, so after replay the
> container stays out of the map until the next stream PutBlock to it;
> re-adding it when a stream entry is applied is left as a follow up.
> Covered by
> {{TestHddsDispatcher#testPutBlockReAddsStreamCreatedContainerAfterRestart}}
> for PutBlock in the apply, stream command and stream link contexts and for
> PutSmallFile in the apply context. Without the change all four fail with the
> IllegalArgumentException from {{updateBCSID}}. With it
> {{TestHddsDispatcher}}, {{TestContainerStateMachineLeader}},
> {{TestContainerStateMachineFollower}}, {{TestKeyValueHandler}},
> {{TestKeyValueHandlerWithUnhealthyContainer}} and {{TestContainerSet}} (177
> tests) and, on a mini cluster, {{TestContainerStateMachineFailures}} and
> {{TestBlockDataStreamOutput}} (29 tests) pass, and checkstyle is clean.
> Found by code review of the datanode streaming PutBlock paths, as part of the
> TLA+ verification effort under HDDS-15926, on commit
> 7212465c0bb19615708c144238317db51c2a696d. 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
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
