[
https://issues.apache.org/jira/browse/HDDS-16840?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16840:
------------------------------
Attachment: MC-1-build-om-membership-requests-from-raft-conf.patch
> 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]
