Siyao Meng created HDDS-17435:
---------------------------------
Summary: Replaying a stream init log entry after a datanode
restart recreates a deleted container replica empty, which SCM can count as
healthy
Key: HDDS-17435
URL: https://issues.apache.org/jira/browse/HDDS-17435
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments:
CR-11-do-not-recreate-missing-container-on-stream-init-replay.patch,
TestBugCR11StreamInitReplayRecreatesDeletedContainer.java
h3. Mechanism
{{HddsDispatcher.dispatchRequest}} treats StreamInit and StreamInitWithPutBlock
as a write stage in every context, so they auto-create a missing container also
when their log entry is applied by {{ContainerStateMachine.applyTransaction}}.
For WriteChunk the write stage is only WRITE_DATA, and the applied stage
(COMMIT_DATA) returns CONTAINER_NOT_FOUND for a missing container, as HDDS-935
intended ("ensure container creation does not happen as part of
applyTransaction of writeChunk request in Ratis"). HDDS-935's missing container
set only covers containers recorded in the last Ratis snapshot, and a replica
deleted by SCM is also removed from the datanode's container table, so a
container created after the last snapshot and deleted since is recreated,
empty, when the stream entry is replayed. For StreamInit, a replayed
CloseContainer entry then closes it.
With PutBlock through Raft the replay then reaches the block's PutBlock entry,
which fails on the missing chunk file and marks the recreated replica
UNHEALTHY, so SCM discards it. With
{{ozone.client.datastream.putblock.without.raft.enabled}} (HDDS-15758,
apache/ozone#10823, master only) the entry is StreamInitWithPutBlock and there
is no PutBlock entry. Its replay recreates the replica and then fails
{{Preconditions.checkArgument}} in {{HddsDispatcher.dispatchRequest}} with an
IllegalArgumentException, because {{applyTransaction}} does not pass
{{container2BCSIDMap}} for that type, so the pipeline is closed and the replica
stays OPEN and empty. After SCM removes the closed pipeline, its close handling
for the mismatched replica quasi closes it and, since every BCSID is 0, force
closes it, so the empty replica ends CLOSED. SCM cannot tell it apart from the
real replicas: {{RatisContainerReplicaCount}} counts any CLOSED replica as
healthy, and {{RatisOverReplicationHandler}} picks excess CLOSED replicas by
{{ContainerReplica.hashCode}}.
h3. Trigger
# Ratis data stream is enabled ({{hdds.container.ratis.datastream.enabled}})
and keys are written with
{{ozone.client.datastream.putblock.without.raft.enabled}}, both off by default.
# A container is created and closed on a pipeline after that pipeline's last
Ratis snapshot.
# SCM deletes the closed replica on one pipeline member while the pipeline
stays open, for example the source of a balancer move, or an excess replica.
# That datanode stops without taking a snapshot (crash or kill) and restarts,
so it replays the stream entries for that container.
h3. Impact
The datanode ends up with an empty CLOSED replica next to the real ones. SCM
counts it as a healthy copy, so redundancy is silently lower than reported, and
when the container is over replicated (for example right after a balancer move)
SCM may delete a real replica instead of the empty one. A mini cluster run of
this sequence on the unfixed commit showed exactly that: after the restart the
replica was OPEN with 0 blocks, after the pipeline was removed it was quasi
closed and then CLOSED with 0 keys and BCSID 0, and ReplicationManager deleted
a real replica and kept the empty one (the key stayed readable from the other
two). That run is not attached; it emulated the balancer move with a replicate
command and a force delete, emulated the crash by restarting the datanode with
the stop snapshot disabled, shortened the pipeline destroy timeout, the scrub,
replication and report intervals to seconds, and chose the victim so that a
real replica sorts first by {{hashCode}}. In released versions (StreamInit with
PutBlock through Raft) the replay also recreates the container, but the replica
ends UNHEALTHY, so they are not affected in the same way.
h3. Reproduction
[^TestBugCR11StreamInitReplayRecreatesDeletedContainer.java]
({{hadoop-hdds/container-service}}) uses a real {{HddsDispatcher}} and
{{ContainerStateMachine}} with a mocked Ratis server. {{stream}} creates the
container, {{KeyValueHandler.deleteContainer}} removes it, a new state machine
with an empty {{container2BCSIDMap}} stands in for a restart from an older
snapshot, and the stream entry and a CloseContainer entry are replayed through
{{applyTransaction}}. On the unfixed commit both cases fail: for StreamInit the
container comes back CLOSED with 0 blocks and the state machine stays healthy;
for StreamInitWithPutBlock it comes back OPEN with 0 blocks and the apply fails
with the IllegalArgumentException described above. With the patch both pass: no
replica is recreated and the apply fails with "ContainerID 1 does not exist". A
failing test means the defect is present. It does not run an SCM delete, a real
restart or ReplicationManager.
h3. Patch
[^CR-11-do-not-recreate-missing-container-on-stream-init-replay.patch], against
7212465c0bb19615708c144238317db51c2a696d. It also applies to master at
ae71e2dd50c. If the patch of HDDS-17430 lands first, this one needs a trivial
rebase in {{TestHddsDispatcher}} (adjacent imports and test method).
The patch counts StreamInit and StreamInitWithPutBlock as a write stage only
when the dispatcher context is not APPLY_TRANSACTION. The stream setup still
creates the container as before, applying the entry for an existing container
is unchanged, and applying it for a missing container now returns
CONTAINER_NOT_FOUND like a replayed WriteChunk or CloseContainer: the state
machine becomes unhealthy and the pipeline is closed, with no replica
recreated. This also removes the IllegalArgumentException on a
StreamInitWithPutBlock replay for a missing container: the precondition is only
reached after an auto create, which no longer happens when the entry is applied.
A stream init entry is written to a peer's Raft log only after that peer's own
{{stream()}} has completed, and {{stream()}} creates the container: Ratis links
the entry to the local stream before writing it ({{ContainerStateMachine.link}}
fails when the stream is missing) and Ozone syncs state machine data before the
log write. So the entry can only be applied for a missing container after the
replica was removed, as for a replayed WriteChunk. The patch reverses the
effect of HDDS-6592, which let the StreamInit replay for a missing container
pass by recreating it. SCM is not changed: once every replica is CLOSED with
BCSID 0 it has no sound signal to prefer one.
Covered by
{{TestHddsDispatcher#testStreamInitCreatesContainerOnlyWhenStreamStarts}} for
both types, with and without {{container2BCSIDMap}} in the apply context.
Without the change it errors with the IllegalArgumentException; with it the
{{HddsDispatcher}}, {{ContainerStateMachine}}, {{KeyValueHandler}} and
{{ContainerSet}} unit suites pass, {{TestBlockDataStreamOutput}} passes, and
checkstyle is clean. The mini cluster sequence above, rerun with the patch,
leaves no replica on the restarted datanode and the three real replicas in
place.
Related: HDDS-17430 (a replayed PutBlock for a container created by a data
stream fails {{updateBCSID}}) is on the same replay path; its fix does not
interact with this one.
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]
