[
https://issues.apache.org/jira/browse/HDDS-17425?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17425:
------------------------------
Affects Version/s: 2.3.0
> Datanode on older software that registers while SCM finalizes stays HEALTHY
> on the finalized SCM and is never asked to register again
> -------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17425
> URL: https://issues.apache.org/jira/browse/HDDS-17425
> Project: Apache Ozone
> Issue Type: Bug
> Affects Versions: 2.3.0
> Reporter: Siyao Meng
> Priority: Minor
> Attachments: MC-1-reregister-older-datanode-after-finalize.patch,
> TestBugMC1OlderDatanodeRegistersDuringFinalize.java
>
>
> h3. Mechanism
> {{SCMBlockProtocolServer.finalizeUpgrade}} checks that every HEALTHY datanode
> runs SCM's software version ({{validateDatanodeVersionsBeforeFinalize}}) and
> then submits the finalization through Ratis. Applying it
> ({{FinalizationStateManagerImpl.finalizeUpgrade}}) finalizes
> {{ScmVersionManager}} and does not re-check registered datanodes.
> {{SCMNodeManager.register}} runs {{shouldFenceDatanode}} once and shares no
> lock with that path, and {{shouldFenceDatanode}} rejects older software only
> once SCM is finalized. So a datanode on older software whose registration is
> handled after the check and before the apply is accepted.
> After the apply nothing checks a HEALTHY node again. On heartbeat,
> {{SCMNodeManager.sendFinalizeToDatanodeIfNeeded}} finds the same fence
> condition but only logs "Invalid datanode in the cluster" and returns,
> {{SCMDatanodeHeartbeatDispatcher}} asks only unknown nodes to register again,
> and the health check does not look at versions. The datanode stays HEALTHY
> and IN_SERVICE, and eligible for pipelines, until it restarts.
> This is the HEALTHY counterpart of HDDS-16798 (a STALE or DEAD older datanode
> that heartbeats back after finalization). The fix for that issue in
> apache/ozone#11692 re-checks only nodes that are not HEALTHY, so it does not
> cover this path.
> h3. Trigger
> # A rolling upgrade from a release that already reports datanode versions
> ({{DatanodeVersionProto}}). Releases that do not report them are rejected at
> registration, so this first applies to upgrades from the first release with
> this code.
> # OM finalizes SCM. The version check passes.
> # A datanode still on the older release starts and its registration reaches
> the SCM before that SCM has applied the finalization. In SCM HA it has to
> reach every SCM before each applies it, because an SCM that has applied it
> rejects the registration and the datanode shuts down. On the leader the
> window is the Ratis round trip of the finalize request.
> In SCM HA the same state is reachable without a timing window (read from
> code, not run). The check reads only the leader's node set and skips nodes
> that are not HEALTHY, so an older datanode that is STALE on the leader but
> HEALTHY on a follower (cut off from the leader only) passes it and stays
> HEALTHY on that follower after the follower applies the finalization; if that
> follower becomes leader, the node is HEALTHY on the leader, where #11692 does
> not re-check it. Even with #11692, a STALE older datanode whose heartbeat
> arrives inside the window becomes HEALTHY without registering, because SCM is
> still not finalized when it checks that heartbeat.
> h3. Impact
> * A datanode that the finalized SCM would reject at registration stays
> HEALTHY and IN_SERVICE and stays in the set that pipeline placement chooses
> from, with two ERROR log lines on every heartbeat, until it restarts. No data
> loss or write failure was shown or is expected from this alone.
> * {{queryUpgradeStatus}} then reports HDDS FINALIZED with the older datanode
> counted as finalized, which is tracked in HDDS-16799.
> h3. Reproduction
> PASS, deterministic, unmodified source.
> [^TestBugMC1OlderDatanodeRegistersDuringFinalize.java]
> ({{hadoop-hdds/server-scm}}) uses the real
> {{SCMBlockProtocolServer.finalizeUpgrade}}, {{SCMNodeManager}} with its
> health check thread, {{SCMDatanodeHeartbeatDispatcher}} and a real
> {{ScmVersionManager}} on a VERSION file at {{ZDU}}. The older datanode runs
> and reports {{ZDU}} (100), SCM and the upgraded datanode run
> {{STREAM_PUTBLOCK_WITHOUT_RAFT}} (101). The {{FinalizationManager}} is
> replaced by a stand in for the Ratis round trip that lets the older
> datanode's registration complete on another thread and then runs
> {{ScmVersionManager.finalizeUpgrade}}. Output on the unmodified code: the
> registration during finalize returns success, the heartbeat after finalize
> logs "Invalid datanode in the cluster ... Datanode software version = ZDU
> (100)" and its reply carries no command, the older datanode is IN_SERVICE
> HEALTHY and in the healthy in service node set, {{queryUpgradeStatus}}
> reports scm=FINALIZED hdds=FINALIZED with 2 of 2 datanodes finalized, and a
> registration of a datanode on the same version after finalize is rejected. A
> passing test means the defect is present. Only the SCM side was run; the
> datanode reaction to the rejection is read from code. The follower route
> above was not run.
> h3. Behaviour change after a forced finalize
> {{ozone admin upgrade finalize --force}} skips the datanode version check.
> Today the older datanodes that are HEALTHY at a forced finalize keep serving
> until they restart. With the patch every one of them is told to register
> again on its next heartbeat (30s by default), is rejected and shuts down, all
> at about the same time (read from code, the datanode side was not run).
> Containers whose replicas are all on older datanodes are unavailable until
> they are re-replicated after those nodes are declared dead, and an EC group
> that loses more datanodes than its parity count is unavailable as well. This
> follows the upgrade rule that all components run the new software when
> finalization is given, but {{--force}} exists to bypass that check, so the
> upgrade owners need to decide whether a forced finalize should remove older
> datanodes right away. SCM does not record whether a finalization was forced,
> so the patch cannot treat the two cases differently without new replicated
> state.
> h3. Suggested fix
> The attached patch adds the re-register on the heartbeat side of the fence
> for HEALTHY nodes and leaves STALE and DEAD nodes to #11692. An alternative
> that covers both in one place is a single check in
> {{SCMDatanodeHeartbeatDispatcher}} for a node of any health, on the software
> version rules only (older software on a finalized SCM, or newer software than
> SCM), which also skips {{processHeartbeat}} for the node that is sent away.
> It must not include the apparent version rule, which an SCM that has not
> applied the finalization yet sees from a correctly finalized datanode
> (HDDS-17090).
> h3. Patch
> [^MC-1-reregister-older-datanode-after-finalize.patch], against
> 679c9ccbee740a0af3ca9f76fc2d4bc6586049eb. It also applies to master at
> 69d77beff60 and on top of apache/ozone#11692.
> When the heartbeat side of the fence finds a datanode on older software than
> SCM and SCM has applied the finalization, {{sendFinalizeToDatanodeIfNeeded}}
> now queues a {{ReregisterCommand}}, which the same heartbeat reply carries.
> The datanode registers again and {{register}} rejects it with
> {{errorNodeNotPermitted}}, as it already does for an older datanode that
> registers after finalization. The rule is extracted into one method that
> {{shouldFenceDatanode}} also uses.
> The re-register is limited to that rule on purpose. The other rule that
> fences on heartbeat, a datanode apparent version above SCM's, is what an SCM
> that has not applied the finalization yet sees from a datanode the leader has
> already finalized (HDDS-17090), so re-registering on it would shut down
> correctly finalized datanodes. The older software rule holds only on an SCM
> that has applied the finalization, and a datanode's software version changes
> only with a restart. With #11692 applied, STALE and DEAD nodes are handled in
> the dispatcher before this code runs, so the two changes act on disjoint
> nodes. Recon is not affected, it overrides {{sendFinalizeToDatanodeIfNeeded}}.
> Covered by {{testOlderDatanodeRegisteredBeforeFinalizeIsAskedToReregister}}
> in the existing {{TestSCMNodeManager}}: a datanode on {{ZDU}} that registered
> while SCM was not finalized gets {{reregisterCommand}} on its heartbeat after
> finalization, and an upgraded datanode gets nothing.
> {{testProcessVersionReportHigherApparentVersion}} now also asserts that a
> datanode whose apparent version is above SCM's gets no command, and its
> comments are narrowed to that case. Without the change the new test fails
> because the reply is empty. With it {{TestSCMNodeManager}},
> {{TestSCMDatanodeHeartbeatDispatcher}}, {{TestSCMBlockProtocolServer}},
> {{TestNodeStateManager}} and {{TestDeadNodeHandler}} (109 tests) pass, and on
> a mini cluster with only this patch applied {{TestScmHAFinalization}}
> (including {{testFinalizedDatanodesShutDownWithPrefinalizedScm}}) and
> {{TestOMUpgradeFinalization}} pass. Checkstyle is clean. With the patch the
> reproduction fails at the assertion on the heartbeat reply, which then
> carries the re-register command. With the patch, an SCM that has not applied
> the finalization still sends no command to a finalized datanode that is STALE
> on it (run on the SCM side).
> Found by TLA+ model checking and code review of the SCM and datanode upgrade
> finalization paths under HDDS-15926, on commit
> 679c9ccbee740a0af3ca9f76fc2d4bc6586049eb. 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]
