[
https://issues.apache.org/jira/browse/HDDS-17438?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17438:
------------------------------
Attachment: CR-10-verify-stream-init-block-token.patch
> Verify the block token of a stream init like WriteChunk, and set up a data
> stream only after its init succeeded
> ---------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17438
> URL: https://issues.apache.org/jira/browse/HDDS-17438
> 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: Critical
> Attachments: CR-10-verify-stream-init-block-token.patch
>
>
> h3. Mechanism
> A data stream write starts with a stream init request ({{StreamInit}}, and on
> master also {{StreamInitWithPutBlock}}) that names its block in
> {{writeChunk.blockID}}. {{ContainerStateMachine.stream}} dispatches the init
> through {{HddsDispatcher}} and then opens the block file for the stream with
> {{HddsDispatcher.getStreamDataChannel}}.
> The invariant this change establishes: a stream init is a block write, so its
> block token is verified like that of a {{WriteChunk}} for the same block (the
> token's block and {{WRITE}} access), and a data stream is set up only when
> its init request succeeded, as {{ContainerStateMachine.streamCommand}}
> already requires for a stream command. {{BlockTokenVerifier}} takes the
> commands that carry a block token, and the block each one names, from
> {{HddsUtils.requireBlockToken}} and {{HddsUtils.getBlockID}}.
> h3. Reproduction
> The issue has no standalone reproduction attached. The regression tests in
> the patch cover the invariant and fail on unmodified source:
> * {{TestBlockTokenVerifier.verifiesStreamInitLikeWriteChunk}}
> ({{hadoop-hdds/framework}}, for both stream init types): a write token for
> the block is accepted for the stream init, and for other tokens the verifier
> reaches the same result for the stream init as for a {{WriteChunk}} of the
> same block.
> * {{ContainerStateMachineTests.testStreamOpenedOnlyIfStreamInitSucceeds}}
> ({{hadoop-hdds/container-service}}, run by
> {{TestContainerStateMachineLeader}} and
> {{TestContainerStateMachineFollower}}, for both stream init types): when the
> init dispatch does not succeed, {{ContainerStateMachine.stream}} fails with
> the init's result and no data channel is opened.
> h3. Patch
> [^CR-10-verify-stream-init-block-token.patch], against
> 7212465c0bb19615708c144238317db51c2a696d. It also applies to master at
> 1acf315d546.
> * {{HddsUtils.requireBlockToken}} lists {{StreamInit}} and
> {{StreamInitWithPutBlock}}, and {{HddsUtils.getBlockID}} returns their
> {{writeChunk.blockID}}, so {{BlockTokenVerifier}} verifies a stream init as
> it verifies a {{WriteChunk}}.
> * {{ContainerStateMachine.getStreamDataChannel}} throws a
> {{StorageContainerException}} with the init's result when the init dispatch
> is not {{SUCCESS}}, so {{stream}} fails before a data channel is opened.
> Compatibility and behaviour:
> * The stock {{BlockDataStreamOutput}} sets the block's token on the stream
> init since 1.4.0 (HDDS-8884), so stream writes of stock clients with block
> tokens enabled pass the check.
> * Every member of the pipeline now verifies the init token when it sets up
> the stream, while a {{WriteChunk}} token is verified once, by the leader. A
> token very close to its expiry, or clock skew between datanodes, can
> therefore fail the stream setup on one member.
> * The stream's Raft request at close carries the init request, so the leader
> also verifies that token at close, as it does for a {{WriteChunk}}. A stream
> open for longer than the token lifetime ({{hdds.block.token.expiry.time}},
> default {{1d}}) fails at close.
> * There is no wire or protocol change and no new configuration.
> * The patch for the stream PutBlock checks, filed separately, also edits the
> stream test setup in {{ContainerStateMachineTests}} ({{setUpStream}}). The
> main code of the two patches merges cleanly; whichever lands second needs a
> small rebase of that test helper. The patch of HDDS-17431 also edits
> {{ContainerStateMachineTests}}; the same applies.
> With the patch {{TestHddsUtils}}, {{TestBlockTokenVerifier}},
> {{TestContainerTokenVerifier}}, {{TestHddsDispatcher}},
> {{TestContainerStateMachineLeader}}, {{TestContainerStateMachineFollower}},
> {{TestKeyValueHandler}} and {{TestKeyValueStreamDataChannel}} pass (171
> tests), {{TestSecureContainerServer}}, {{TestBlockDataStreamOutput}} and
> {{TestOzoneAtRestEncryption}} in {{hadoop-ozone/integration-test}} (mini
> clusters with data streaming, the last one with block tokens and stream keys
> written by the stock client) pass (58 tests), and checkstyle on
> {{hadoop-hdds/common}}, {{hadoop-hdds/framework}} and
> {{hadoop-hdds/container-service}} 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. Related: HDDS-8884.
> 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]