[ 
https://issues.apache.org/jira/browse/HDDS-16838?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng updated HDDS-16838:
------------------------------
    Attachment: MC-6-check-bootstrapping-om-config-has-all-ring-members.patch

> OM bootstrap check passes when the new OM's own config lacks a ring member; 
> the OM is added as a voter and then stops with an NPE
> ---------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HDDS-16838
>                 URL: https://issues.apache.org/jira/browse/HDDS-16838
>             Project: Apache Ozone
>          Issue Type: Bug
>            Reporter: Siyao Meng
>            Priority: Major
>         Attachments: 
> MC-6-check-bootstrapping-om-config-has-all-ring-members.patch, 
> TestBugMC6BootstrapOmittedMember.java
>
>
> h3. Mechanism
> {{OzoneManager.checkConfigBeforeBootstrap}} asks every OM in the new OM's own 
> config whether it knows the new OM. It never checks the other direction, that 
> the new OM's config has every member of the ring, although each existing OM 
> returns its peer list in the same reply 
> ({{OMConfiguration.getCurrentPeerList}}).
> A bootstrapping OM starts with an empty Ratis peer list and fills it from the 
> Raft configurations it is notified of. For a member without 
> {{ozone.om.address.<serviceId>.<nodeId>}} in the local config, 
> {{OzoneManager.addOMNodeToPeers}} logs "There is no OM configuration for node 
> ID ... in ozone-site.xml." and skips it. When the configuration that contains 
> the new OM itself is applied, the branch of {{OzoneManager.updatePeerList}} 
> for the local node calls 
> {{omRatisServer.addRaftPeer(peerNodesMap.get(peerNodeId))}} with null for 
> that member. The {{NullPointerException}} is thrown on the Ratis 
> {{StateMachineUpdater}} thread, Ratis closes the state machine and 
> {{OzoneManagerStateMachine.close}} terminates the OM with exit status 0. At 
> that point the leader has already committed the new OM as a voter and the new 
> OM has logged "Successfully bootstrapped OM".
> Both the check and the unguarded {{addRaftPeer}} call are in the release tags 
> from 1.3.0 (HDDS-5534) to 2.2.1, and not in 1.2.x.
> h3. Trigger
> # Ring of omNode-1 (leader) and omNode-2. The new node is added to the config 
> of both.
> # The {{ozone-site.xml}} of the new node has no entry for omNode-2 (not in 
> {{ozone.om.nodes.<serviceId>}} and no 
> {{ozone.om.address.<serviceId>.omNode-2}}).
> # {{ozone om --bootstrap}} on the new node. The check passes because only 
> omNode-1 is asked. The leader adds the node, then the node stops with the NPE.
> No fault and no {{--force}} is needed, but the config of the new node has to 
> differ from the one on the existing OMs, which the documented procedure (the 
> same updated {{ozone-site.xml}} on all OMs) does not produce. If only the 
> nodes list omits the member and its address key is present, the bootstrap 
> works (run).
> h3. Impact
> * The new OM is a voter in the committed Raft configuration while its process 
> is down, after it logged a successful bootstrap and exited with status 0. A 
> ring of two becomes three voters with two running, so it keeps a leader but 
> tolerates no further failure (run, a write still succeeds).
> * A second new OM started the same way, with a config that also lacks the 
> first new OM (otherwise the check fails because that OM is down), gives four 
> voters with two running. The leader steps down with 
> {{LOST_MAJORITY_HEARTBEATS}}, no leader is elected and a client write fails 
> (run). On a ring of three the numbers differ (not run): one such join leaves 
> four voters with three running, so the ring that tolerated one failure 
> tolerates none, and it takes three such joins (six voters, three running) to 
> lose the leader.
> * Recovery: starting a stopped OM normally with the complete config brings it 
> back. In the two join case a leader was ready within a few seconds of that 
> start on a mini cluster (0.5 to 4 s over five runs). A restart with the 
> unchanged config was not run.
> h3. Reproduction
> PASS, no fault injection and no timing hooks, unmodified source. 
> [^TestBugMC6BootstrapOmittedMember.java] ({{hadoop-ozone/integration-test}}) 
> uses the real OMs of a mini cluster and the plain {{BOOTSTRAP}} startup 
> option, about 60 seconds. The new OM is created the way 
> {{MiniOzoneHAClusterImpl.bootstrapOzoneManager}} does it, except that its own 
> config differs from the config of the running OMs. The first test shows one 
> join (three members in the Raft configuration, the two log lines, the NPE, 
> exit status 0, and the recovery by a normal start). The second shows two 
> joins leaving the ring without a leader, a failed write, and the recovery. A 
> passing test means the defect is present. The first test passed in every run. 
> The second passed in five of six runs: on a busy host Ratis once refused the 
> second join ({{RATIS_BOOTSTRAP_ERROR}}, "Fail to set configuration ... due to 
> NOPROGRESS") because the new OM did not answer the leader within the short 
> timeouts of the mini cluster, and the test ended there with an error.
> {noformat}
> INFO  om.OzoneManager - Successfully bootstrapped OM omNode-bootstrap-1 and 
> joined the Ratis group
> ERROR impl.StateMachineUpdater - 
> [email protected] caught a Throwable.
> java.lang.NullPointerException: Cannot invoke 
> "org.apache.hadoop.ozone.om.helpers.OMNodeDetails.getNodeId()" because 
> "omNodeDetails" is null
>       at 
> org.apache.hadoop.ozone.om.ratis.OzoneManagerRatisServer.addRaftPeer(OzoneManagerRatisServer.java:461)
>       at 
> org.apache.hadoop.ozone.om.OzoneManager.updatePeerList(OzoneManager.java:2338)
>       at 
> org.apache.hadoop.ozone.om.ratis.OzoneManagerStateMachine.notifyConfigurationChanged(OzoneManagerStateMachine.java:340)
> INFO  om.OzoneManager - Terminating with exit status 0: OM state machine is 
> shutdown by Ratis server
> {noformat}
> h3. Patch
> [^MC-6-check-bootstrapping-om-config-has-all-ring-members.patch], against 
> 7fcf31294859d5d31163e016b5a9a63f21c6edd2. It also applies to master at 
> 1cc6423590f (not built there).
> {{checkConfigBeforeBootstrap}} now also goes through the peer list each 
> existing OM reports ({{OMConfiguration.getCurrentPeerList}}, the in memory 
> peer list of that OM, not the Raft configuration) and exits with "OM(s) [...] 
> are in the peer list of the existing OMs but have no address in the 
> configuration of the bootstrapping OM. Update its ozone-site.xml before 
> proceeding." before the leader is asked. It uses only the address lookup that 
> {{addOMNodeToPeers}} starts with 
> ({{OMNodeDetails.getOMNodeAddressFromConf}}), so a new OM whose config has an 
> address for every listed peer is not refused. An address that is present but 
> cannot be turned into a socket address is not covered and still reaches the 
> NPE (from the code, not run). No request or protocol change.
> One bootstrap that works today is refused with the patch (from the code, not 
> run): an existing OM that was started with a node in its {{ozone.om.nodes}} 
> list that is not in the ring keeps that node in its in memory peer list until 
> it applies the next Raft configuration change, and a new OM whose config has 
> no address for that node is refused in that window. It needs the config files 
> to disagree.
> Covered by {{testBootstrapWithExistingOMMissingInNewOMConfig}} in the 
> existing {{TestAddRemoveOzoneManager}}, which also asserts that no existing 
> OM has the new OM in its Raft configuration afterwards. Without the change it 
> fails because {{start()}} does not throw. With it the nine tests of 
> {{TestAddRemoveOzoneManager}} pass, checkstyle is clean, and both tests of 
> the reproduction fail at their first bootstrap with the new message.
> Not changed: {{--force}} skips the whole check, so the NPE can still be 
> reached with it. A null check in {{updatePeerList}} alone would keep the OM 
> running without the member in its peer lists, so it is not part of this patch.
> Found by TLA+ model checking and code review of the OM HA membership change 
> paths (bootstrap, decommission and leader transfer) under HDDS-15926, on 
> commit 7fcf31294859d5d31163e016b5a9a63f21c6edd2. 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
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to