[
https://issues.apache.org/jira/browse/HDDS-17550?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17550:
------------------------------
Attachment: MC-2-require-known-leader-for-local-lease-read.patch
> OM local lease check never detects a missing leader, so a leaderless OM or a
> not ready leader can serve LOCAL_LEASE reads from its local DB
> -------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17550
> URL: https://issues.apache.org/jira/browse/HDDS-17550
> Project: Apache Ozone
> Issue Type: Bug
> Affects Versions: 2.2.0, 2.2.1
> Reporter: Siyao Meng
> Priority: Major
> Attachments: MC-2-require-known-leader-for-local-lease-read.patch
>
>
> h3. Mechanism
> {{OzoneManagerProtocolServerSideTranslatorPB.allowFollowerReadLocalLease}}
> decides whether an OM may answer a read from its local DB without a Raft
> round trip. It is meant to answer no for a node that is not a follower and
> for a follower that has no leader, then to check the time since the last RPC
> from the leader against {{ozone.om.follower.read.local.lease.time.ms}}, and
> finally to compare the node's applied index with the leader's commit index
> using {{ozone.om.follower.read.local.lease.log.limit}}. A log limit of -1
> skips the index comparison.
> The two presence checks are written as {{followerInfo == null}} and
> {{leaderInfo == null}}. Protobuf getters never return null, they return the
> default instance, so neither check can fail. What Ratis puts into
> {{RoleInfoProto}} ({{RoleInfo.buildRoleInfoProto}}, the same in Ratis 3.2.1
> and 3.3.1):
> * LEADER: only {{leaderInfo}}. {{getFollowerInfo()}} is the default instance.
> * CANDIDATE: only {{candidateInfo}}. Same result.
> * FOLLOWER (and LISTENER): {{followerInfo}} with {{leaderInfo}} always set.
> When the leader is unknown, {{leaderInfo}} is
> {{ServerRpcProto.getDefaultInstance()}}
> ({{ServerProtoUtils.toServerRpcProto(null, ...)}}), which has no {{id}}. A
> follower is without a leader after its election timeout, after granting its
> vote to a candidate, and until the first append from a new leader.
> In all three cases without a known leader, the elapsed time read from
> {{leaderInfo}} is 0, so the time check passes. Then:
> * *Node without a known leader (CANDIDATE, or FOLLOWER with no leader).* With
> the server's log limit set to -1 the method returns true before it looks for
> a leader, and the read is served from the local DB. With a finite limit it
> returns false, because {{getLeaderId()}} is null and no leader commit index
> is found.
> * *Leader that is not ready yet.* {{submitReadRequestToOmLocalLease}} sends
> LEADER_AND_NOT_READY down the same branch as NOT_LEADER, and the path for
> requests that name no read consistency calls the method for any role. For a
> leader {{getLeaderId()}} is the node itself and {{getCommitInfos()}} lists
> its own commit index first, so the index comparison checks the node against
> itself and passes. Until its first entry of the term commits, a new leader
> does not know the cluster's commit index. After a restart the gap can be
> large: OM disables Ratis log metadata ({{OzoneManagerRatisServer}},
> {{setLogMetadataEnabled(properties, false)}}), so {{RaftLogBase}} starts the
> commit index at the state machine snapshot index, which is the OM DB
> {{TransactionInfo}}. Transactions acknowledged to clients but not yet flushed
> by the double buffer before the OM went down are above that index.
> For comparison, the DEFAULT and LINEARIZABLE_LEADER_ONLY paths
> ({{submitReadRequestToOmDefault}},
> {{submitReadRequestToOmLinearizableLeaderOnly}}) refuse a not ready leader
> with {{OMLeaderNotReadyException}}, LINEARIZABLE_ALLOW_FOLLOWER waits in the
> Ratis ReadIndex until the leader is ready ({{LeaderStateImpl.getReadIndex}}),
> and write requests are refused through
> {{OzoneManagerRatisUtils.checkLeaderStatus}}.
> h3. Trigger
> Requires {{ozone.om.follower.read.local.lease.enabled=true}} and LOCAL_LEASE
> reads.
> * *Node without a known leader:* the server's log limit is -1 and the OM has
> lost its leader (election timeout without a new leader, or between granting
> its vote and the first append from the new leader).
> * *Not ready leader:* any log limit. An OM wins an election and a LOCAL_LEASE
> read reaches it before its first entry of the term is applied, which is
> normally one replication round. The read misses acknowledged writes when the
> OM's applied index is behind the cluster's committed index by more than the
> log limit. One leader crash is enough for a gap of a few entries (the new
> leader does not yet know the old leader's last commit). A large gap needs the
> elected OM's DB to be behind its committed log at restart, for example a
> double buffer backlog lost when the OM went down; a whole cluster restart
> after a power loss is one such event. Every variant needs a read inside the
> not ready window.
> h3. Impact
> * A leaderless OM answers LOCAL_LEASE reads from its local DB although the
> time check is meant to refuse a node that has not heard from a leader
> recently.
> * A newly elected, not ready leader answers LOCAL_LEASE reads checked only
> against itself. In a three OM mini cluster run with the server log limit at 0
> (not attached), an OM restarted with a bucket create in its Raft log but not
> in its DB, then elected and held in the not ready window with the Ratis
> {{LeaderState.placeholder}} test hook, answered {{getBucket}} for that
> acknowledged bucket with {{BUCKET_NOT_FOUND}}.
> * Nothing is written and no data is lost; the reader gets an older state.
> Not changed by the patch: a follower of a newly elected leader compares
> itself with that leader's commit index. Until the new leader's first entry of
> the term commits, that commit index can be as low as the leader's DB snapshot
> index after a restart, so such a follower can also answer without the
> unflushed backlog for that window. It is within the documented wording of
> LOCAL_LEASE ("compared to the leader's commit index") but has the same deeper
> cause; it may deserve its own issue.
> h3. Reproduction
> No standalone reproduction is attached. The regression test in the patch
> shows the behaviour on a three OM mini cluster, with the server log limit set
> to -1 through {{OmConfig}} and the not ready window held with the Ratis
> {{LeaderState.placeholder}} injection point: without the change the not ready
> leader and the leaderless OM both answer from their local DB ("Expected
> com.google.protobuf.ServiceException to be thrown, but nothing was thrown").
> h3. Patch
> [^MC-2-require-known-leader-for-local-lease-read.patch], against
> efe0c7eba1527530d1001f6517b733576fdc6d61. It also applies to master at
> efe0c7eba15.
> {{allowFollowerReadLocalLease}} now uses real presence checks on one
> {{RoleInfoProto}}: {{hasFollowerInfo()}} for "not a follower" and {{hasId()}}
> on the follower's {{leaderInfo}} for "no leader", both before the time check
> and the -1 shortcut. A CANDIDATE, a FOLLOWER without a leader and a LEADER
> are refused. The callers already turn a refusal into {{OMNotLeaderException}}
> for NOT_LEADER and {{OMLeaderNotReadyException}} for LEADER_AND_NOT_READY, so
> a not ready leader now answers LOCAL_LEASE like the DEFAULT and
> LINEARIZABLE_LEADER_ONLY paths do.
> For requests that name no read consistency, a ready leader keeps its current
> behaviour: the patch looks up the leader status first and lets a ready leader
> take the local read as before ({{raftServerStatus == LEADER_AND_READY ||
> allowFollowerReadLocalLease(...)}}), matching the LEADER_AND_READY case of
> {{submitReadRequestToOmLocalLease}}. The only remaining difference is that a
> ready leader whose apply backlog is above a finite log limit now also reads
> locally instead of falling back to its leader read, which is at least as
> fresh. A follower that knows its leader is checked exactly as before: it
> reports the real time since the leader's last RPC, and the time check, the -1
> shortcut and the index comparison run unchanged.
> Landing order: the client fix in HDDS-17548 should land first or together
> with this one. This patch makes OMs refuse LOCAL_LEASE reads they used to
> answer (a leaderless OM at log limit -1, and every not ready leader), and a
> client without HDDS-17548 reacts to such a refusal by turning follower read
> off for the rest of its life or by retrying the same not ready OM. 2.2.x
> clients without that fix keep this behaviour against a fixed server.
> Covered by
> {{TestOzoneManagerHAFollowerReadWithStoppedNodes.testLocalLeaseReadRequiresKnownLeader}}:
> with the server log limit at -1, a ready leader and a follower with a known
> leader answer from their local DB; then the next leader is held in the not
> ready window and refuses with {{OMLeaderNotReadyException}}, and the other
> OM, which has no known leader, does not answer locally and fails in the
> linearizable read with {{ReadIndexException}}. With the change
> {{TestOzoneManagerHAFollowerReadWithAllRunning}},
> {{TestOzoneManagerHAFollowerReadWithStoppedNodes}},
> {{TestOzoneManagerHAWithAllRunning}} and {{TestOzoneShellHAWithFollowerRead}}
> pass (56 tests) and checkstyle is clean. With the HDDS-17548 patch applied on
> top, both build and the two follower read suites and the client follower read
> proxy provider tests pass.
> Found by TLA+ model checking and code review of the OM follower read and read
> consistency routing paths under HDDS-15926, on commit
> efe0c7eba1527530d1001f6517b733576fdc6d61. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing. Related but not
> duplicates: HDDS-16435 (LINEARIZABLE_ALLOW_FOLLOWER on a superseded leader, a
> different read path), HDDS-16758 (a test that checked the wrong OM's local
> lease metrics) and HDDS-17548 (client reaction to a refused LOCAL_LEASE
> read). 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]
