[ 
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]

Reply via email to