Siyao Meng created HDDS-17418:
---------------------------------
Summary: 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
Reporter: Siyao Meng
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]
