[
https://issues.apache.org/jira/browse/HDDS-17435?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17435:
------------------------------
Attachment:
CR-11-do-not-recreate-missing-container-on-stream-init-replay.patch
> 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
> Affects Versions: 2.3.0
> Reporter: Siyao Meng
> Priority: Major
> 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]
