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

Siyao Meng updated HDDS-16840:
------------------------------
    Attachment: TestBugMC1DecommissionAfterBootstrapDropsNewOM.java

> OM bootstrap and decommission can drop other OMs or change their Raft role
> --------------------------------------------------------------------------
>
>                 Key: HDDS-16840
>                 URL: https://issues.apache.org/jira/browse/HDDS-16840
>             Project: Apache Ozone
>          Issue Type: Bug
>            Reporter: Siyao Meng
>            Priority: Major
>         Attachments: MC-1-build-om-membership-requests-from-raft-conf.patch, 
> TestBugMC1DecommissionAfterBootstrapDropsNewOM.java, 
> TestBugMC2MembershipChangeFlipsRaftRole.java, 
> TestBugMC4ForceBootstrapUnknownVoter.java
>
>
> h3. Mechanism
> {{OzoneManagerRatisServer.addOMToRatisRing}} and {{removeOMFromRatisRing}} 
> send Ratis the complete new member list in a {{SetConfigurationRequest}} with 
> the default mode ({{SET_UNCONDITIONALLY}}). They build the list from 
> {{raftPeerMap}}, the leader's own peer list, and nothing compares it with the 
> Raft configuration. Whatever the leader's list says about the other members 
> is committed. The list can differ from the Raft configuration in three ways:
> * It lags. {{raftPeerMap}} changes only when the leader's state machine 
> applies a configuration entry 
> ({{OzoneManagerStateMachine.notifyConfigurationChanged}}, 
> {{OzoneManager.updatePeerList}}). Ratis answers a membership change and 
> accepts the next one when the entry is committed.
> * Its roles are local. Whether a peer is a voter or a listener comes from 
> {{ozone.om.listener.nodes}} in the leader's own config 
> ({{OzoneManager.addOMNodeToPeers}}), not from the configuration entry. 
> {{OzoneManager.checkRemoteOMConfig}} does not compare the listener flag, so a 
> bootstrap passes when the joining OM and the running OMs disagree.
> * It can miss a member. Since HDDS-8872, {{OzoneManager.addOMNodeToPeers}} 
> logs an error and returns when the local config has no address for a node 
> named in an applied entry. {{OMAdminProtocolServerSideImpl.decommission}} 
> looks the node up in the same local view ({{OzoneManager.getPeerNode}}).
> h3. Trigger
> Each symptom needs a bootstrap ({{ozone om --bootstrap}}) or a decommission 
> ({{ozone admin om decommission}}) handled by a leader whose peer list differs 
> from the Raft configuration.
> # Timing. A decommission is handled after an earlier membership change has 
> committed and before the leader has applied it. A membership change writes 
> two configuration entries. The leader updates its list when it applies the 
> first, and Ratis replies when the second is committed. In three bootstraps 
> measured on a mini cluster the leader's list was updated 11 to 23 ms after 
> the reply. In production the apply step also reloads the config files and 
> resolves the new address on the same thread (code reading). The decommission 
> CLI starts a JVM, checks the config of every OM and sends one request without 
> retry, so two commands typed one after the other should not hit this (argued, 
> the CLI was not run). A caller that sends the second request right after the 
> first reply can hit it. The failure reported in HDDS-14017 fits this window, 
> see below.
> # Roles. {{ozone.om.listener.nodes.<serviceId>}} differs between a joining OM 
> and the running OMs when it bootstraps. The listener design asks for the same 
> value on all OMs, but nothing checks it and a normal bootstrap without 
> {{--force}} passes.
> # Forced bootstrap. An OM joins with {{ozone om --bootstrap --force}} while 
> the config of the running OMs does not name it. The HA documentation 
> describes {{--force}} for the case that an existing OM is down and warns 
> about OMs that do not have the updated config.
> h3. Impact
> All three were reproduced on unmodified source with real OMs in a mini 
> cluster. No state is injected.
> * *A decommission also removes the OM that was just bootstrapped* (trigger 
> 1). Both operations report success. The Raft configuration goes from four 
> voters to two and the new OM shuts itself down. Only this direction was 
> reproduced. The other direction (a stale list that adds a removed OM back) 
> was not run here. In the three OM ring of HDDS-14017 such a request was 
> rejected by Ratis because it "changes a majority set of the current 
> configuration": the request quoted there still names the OM that the 
> configuration in the same message no longer has. With three or more voters 
> left that check does not reject it and Ratis would start to add the removed 
> OM again (code reading of {{RaftConfigurationImpl.changeMajority}}, not run). 
> Until HDDS-16084, the wait between the two decommissions in 
> {{TestAddRemoveOzoneManager.testDecommission}} compared a list of 
> {{OMNodeDetails}} with a {{String}} and returned at once, so the second 
> request could reach the leader inside the window. 1 failure in 200 runs is 
> reported there. The issue is linked to HDDS-13768 as the change with which 
> the failures were first seen. That change does not touch the membership code. 
> All of this is read from the issues and the sources, not rerun. Affects 1.3.0 
> and later, where decommission was added (read from the release tags, not run 
> there). [^TestBugMC1DecommissionAfterBootstrapDropsNewOM.java]
> * *A later membership change flips the role of another OM* (trigger 2). An OM 
> that joined as a voter while the running OMs list it as a listener is demoted 
> to listener by the next bootstrap of a different OM. An OM that joined as a 
> listener while the running OMs do not list it is promoted to voter by the 
> next decommission of a different OM. Both are committed Raft configurations 
> and both operations report success. Affects 2.1.0 and later, where listeners 
> were added (read from the release tags). 
> [^TestBugMC2MembershipChangeFlipsRaftRole.java]
> * *A voter the leader does not know cannot be decommissioned and is expelled 
> by the next bootstrap* (trigger 3). After the forced bootstrap, and after the 
> config of the running OMs has been corrected, the decommission of that OM is 
> refused with "OM node not present in the OM peer list.". The normal bootstrap 
> of another OM then commits a configuration without it, and it shuts down. 
> Affects 1.4.0 and later, where HDDS-8872 is included (read from the release 
> tags). [^TestBugMC4ForceBootstrapUnknownVoter.java]
> In all three cases the ring ends with fewer voters or with other voters than 
> the operator asked for and was told. No data is lost by the mechanism. What 
> that means for clients, for example writes that fail after one more OM stops, 
> was not exercised here.
> h3. Reproduction
> The three attached classes are self contained, go into 
> {{hadoop-ozone/integration-test}} (target path and Maven command are in each 
> class comment) and pass on unmodified source when the defect is present. Each 
> starts three OMs in a mini cluster and uses the entry points behind the 
> commands: an OM started in bootstrap mode 
> ({{MiniOzoneHAClusterImpl.bootstrapOzoneManager}}) and 
> {{OMAdminProtocol.decommission}}. The CLI itself is not run.
> Only the first class uses a timing aid. It holds the leader while it applies 
> the configuration entry of the bootstrap: the config object given to the 
> leader through {{OzoneManager.setConfiguration}} blocks the lookup of the new 
> OM's address on the Ratis thread that applies entries until the decommission 
> has returned. The decommission is sent after the new OM has logged that its 
> bootstrap succeeded. So this class shows that the window exists, not how 
> often it is hit. The other two only prepare config (the listener key, or no 
> entry for the new node) and time nothing.
> h3. Patch
> [^MC-1-build-om-membership-requests-from-raft-conf.patch], against 
> 7fcf31294859d5d31163e016b5a9a63f21c6edd2. It also applies to master at 
> 1cc6423590f (not built there).
> * {{addOMToRatisRing}} and {{removeOMFromRatisRing}} read the members and 
> their roles from the Raft configuration 
> ({{getServerDivision().getRaftConf()}}) and add or remove only the requested 
> node. The other members are written with the default priority, as before.
> * The request uses the Ratis {{COMPARE_AND_SET}} mode with the configuration 
> the lists were derived from as the expected one. Ratis compares the ids of 
> voters and listeners with its current configuration under the lock that 
> starts the reconfiguration, so the request is rejected with 
> {{SetConfigurationException}} if the members or their roles have changed 
> since the lists were derived. A change of only an address or a priority is 
> not noticed. {{Mode.ADD}} would be less code for the add, but it does not 
> cover the remove and it accepts a node that is already a member.
> * {{removeOMFromRatisRing}} takes the node id and checks it against the Raft 
> configuration, with the same error text as before. The lookup in 
> {{OMAdminProtocolServerSideImpl.decommission}} is removed, so a member the 
> leader has no config for can be decommissioned.
> No request or proto field changes. The Ratis request is built and consumed on 
> the leader.
> Visible changes besides the symptoms: members that are not the target keep 
> the address they have in the Raft configuration (before, it was written again 
> from the leader's config), a request that races another membership change now 
> fails where it used to overwrite, and the decommission request checks admin 
> rights before it checks membership. The patch does not change how the leader 
> maintains its own peer list, so that list can still lag, carry local roles or 
> miss a member. It is no longer used to build membership requests.
> Covered by five cases in existing test classes. 
> {{TestOzoneManagerRatisServer}}: 
> {{testRemoveKeepsMemberMissingFromLocalPeerList}}, 
> {{testRemoveKeepsRoleOfOtherMembers}}, {{testRemoveRejectsSelfAndNonMember}} 
> and {{testRemoveWritesDefaultPriority}}. {{TestAddRemoveOzoneManager}}: 
> {{testBootstrapKeepsOMMissingFromLeaderPeerList}}, a forced bootstrap without 
> config update followed by a normal bootstrap of a second OM, after which both 
> must be in the ring. On unmodified source (test calls adapted to the old 
> method signature) the first three fail, the third only because the membership 
> check was in the RPC handler, and the mini cluster case fails with a timeout 
> in the cluster helper, which waits for the expelled OM. The priority case 
> states existing behaviour and was not run on unmodified source. With the 
> patch {{TestOzoneManagerRatisServer}} (11 tests), 
> {{TestOMAdminProtocolServerSideImpl}} (1 test) and 
> {{TestAddRemoveOzoneManager}} (9 tests) pass and checkstyle is clean. With 
> the patch the three attached classes fail at the assertions that describe the 
> defect.
> 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 and 
> RATIS issues and apache/ozone pull requests for duplicates before filing. 
> Related: HDDS-14017 (open, the flaky test failure described above, which fits 
> the first trigger), HDDS-12648 (resolved, removed a leader status call 
> without effect from the same two methods), HDDS-8872 (resolved, made a 
> missing config entry non fatal when a configuration entry is applied) and 
> HDDS-7584 (resolved, a new OM expelled itself after a restart by replaying an 
> old removal). 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