[
https://issues.apache.org/jira/browse/HDDS-17418?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17418:
------------------------------
Attachment: TestBugCR3WriteVersionFirstNode.java
> Pipeline write version uses the first datanode's version, so writes on looked
> up pipelines can carry a version above the lowest member
> --------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17418
> URL: https://issues.apache.org/jira/browse/HDDS-17418
> Project: Apache Ozone
> Issue Type: Bug
> Affects Versions: 2.3.0
> Reporter: Siyao Meng
> Priority: Minor
> Attachments:
> CR-3-use-lowest-member-version-as-pipeline-write-version.patch,
> TestBugCR3WriteVersionFirstNode.java
>
>
> h3. Mechanism
> {{Pipeline.getWriteVersion}} returns the serialized current version of the
> first member. Its Javadoc assumes all members share one version. That holds
> for pipelines from block allocation, where
> {{ScmBlockLocationProtocolServerSideTranslatorPB.allocateScmBlock}} sets the
> minimum member version on every member. It does not hold for pipelines from a
> container lookup:
> {{StorageContainerLocationProtocolServerSideTranslatorPB.getContainerWithPipeline}}
> (and the batch variants) set each member's own live version since
> HDDS-16044. {{ContainerProtocolCalls}} copies {{getWriteVersion}} into
> {{writePipelineVersion}} for FinalizeBlock, PutBlock, WriteChunk and
> PutSmallFile, and {{BlockDataStreamOutput}} does the same for StreamInit.
> h3. Trigger
> # During a rolling upgrade, a Ratis pipeline has members at versions 101, 100
> and 100, with the upgraded datanode listed first.
> # Lease recovery runs on a hsync'ed key.
> {{OMRecoverLeaseRequest.updateBlockInfo}} replaces the last block's pipeline
> with the result of {{getContainerWithPipeline}}.
> # The file system client calls {{finalizeBlock}}
> ({{BasicRootedOzoneClientAdapterImpl}} or {{BasicOzoneClientAdapterImpl}}),
> which sends FinalizeBlock with {{writePipelineVersion}} 101.
> Argued, not run: {{XceiverClientManager}} caches the client by pipeline ID
> and type (plus the user in secure mode). If the process had no write client
> cached for that pipeline yet, the client created for FinalizeBlock keeps the
> looked up pipeline, and later WriteChunk and PutBlock calls from the same
> process to that pipeline take the version from it. If a write client was
> already cached, FinalizeBlock reuses it and its allocation pipeline, and
> sends the right value.
> h3. Impact
> None observable at this commit. The datanode stores the value in
> {{DispatcherContext}} ({{KeyValueHandler}}, {{ContainerStateMachine}}), but
> no main code calls {{DispatcherContext.getWriteVersion}}. The client
> PutBlock-without-Raft check
> ({{BlockDataStreamOutput.allDatanodesSupportStreamPutBlock}}) checks each
> member itself, and FinalizeBlock has no version gate. It breaks the rule that
> a write pipeline never uses a version above its lowest member, and any
> datanode side gate that starts reading the write version would let such
> writes through.
> h3. Reproduction
> PASS on unmodified source. [^TestBugCR3WriteVersionFirstNode.java]
> ({{hadoop-hdds/common}}) serializes one pipeline through both SCM paths,
> parses it as the client does, builds a FinalizeBlock request with
> {{ContainerProtocolCalls.finalizeBlock}} (only the transport is mocked), and
> derives the datanode write version with
> {{ClientCommandsUtils.getWritePipelineVersion}}. Output: allocation pipeline
> 100; looked up pipeline 101 with lowest member 100; FinalizeBlock
> {{writePipelineVersion}} 101, datanode write version
> {{STREAM_PUTBLOCK_WITHOUT_RAFT}} (101). A passing test means the defect is
> present. No cluster was run with datanodes on two builds.
> h3. Patch
> [^CR-3-use-lowest-member-version-as-pipeline-write-version.patch], against
> 5f7111a27ea07ce6b7bf5284a4213ebacb65f2ae. It also applies to master at
> e03ba37c41d.
> {{getWriteVersion}} returns the lowest serialized member version. Negative
> values (unknown future versions, as in {{ComponentVersion.isSupportedBy}})
> rank above all known versions, so a plain integer minimum is not used. The
> raw serialized value is still forwarded. Allocation pipelines get the same
> value as before.
> New tests in the existing {{TestPipeline}} cover a newest first member,
> members with unknown versions, and a pipeline whose members are all
> {{UNKNOWN_VERSION}} (unchanged result). Without the change both fail
> ({{expected: <100> but was: <101>}} and {{expected: <100> but was: <-1>}}).
> {{TestContainerProtocolCalls.writeVersionPreservesOriginalSerializedValueFromWire}}
> relied on first member selection and now sets the undeserializable version
> on every member.
> With the change, {{TestPipeline}}, {{TestContainerProtocolCalls}},
> {{TestClientCommandsUtils}}, {{TestDatanodeDetails}}, the client
> {{TestBlockDataStreamOutput}}, {{TestKeyValueHandler}} and
> {{TestDispatcherContext}} pass, as do {{TestDatanodeCurrentVersionEndToEnd}}
> and {{TestBlockDataStreamOutput}} on a mini cluster. Checkstyle is clean.
> With the patch, the reproduction fails because the request carries 100.
> Found by code review of the zero downtime upgrade version gating in SCM, OM
> and the datanode write path, as part of the TLA+ verification effort under
> HDDS-15926, on commit 5f7111a27ea07ce6b7bf5284a4213ebacb65f2ae. 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]
