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