bitflicker64 commented on code in PR #3376:
URL: https://github.com/apache/hugegraph/pull/3376#discussion_r4328414737
##########
hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java:
##########
@@ -650,6 +668,74 @@ public Node getRaftNode() {
return raftNode;
}
+ /**
+ * The last index of this node's raft log when the node started. After a
restart jraft
+ * applies the log after the last snapshot again, and every entry at or
below this index
+ * is such a replay; an entry above it is new in this run. Long.MAX_VALUE
while no node
+ * runs or the index could not be read, so every entry counts as a replay
then.
+ */
+ public long getLastLogIndexAtStart() {
+ return this.lastLogIndexAtStart;
+ }
+
+ /**
+ * Fails when a peer of the current configuration is not confirmed to
apply the TXN
+ * entry, see {@link TxnPeerGate}; does nothing without a node
+ *
+ * @throws PDException TXN_NOT_PROPOSED
+ */
+ public void checkTxnPeers() throws PDException {
+ TxnPeerGate gate = this.txnPeerGate;
+ if (gate != null) {
+ gate.check();
+ }
+ }
+
+ /**
+ * Fails when a peer of a configuration about to be installed is not
confirmed to apply
+ * the TXN entry: a peer that joins replays the log, TXN entries included.
Called before
+ * every membership change; does nothing without a node
+ *
+ * @throws PDException TXN_NOT_PROPOSED, naming the peer
+ */
+ public void checkTxnPeers(Configuration next) throws PDException {
+ TxnPeerGate gate = this.txnPeerGate;
+ if (gate == null) {
+ return;
+ }
+ PeerId self = this.config == null ? null :
JRaftUtils.getPeerId(this.config.getAddress());
+ List<String> endpoints = new ArrayList<>();
+ List<PeerId> peers = new ArrayList<>(next.getPeers());
+ peers.addAll(next.getLearners());
+ for (PeerId peer : peers) {
+ if (!PeerUtil.isPeerEquals(peer, self)) {
+ endpoints.add(peer.getEndpoint().toString());
+ }
+ }
+ gate.confirm(endpoints);
Review Comment:
Important: This passes every peer of the new configuration to
`gate.confirm`, including the ones already in the current configuration, so a
membership change is now refused whenever a peer it keeps does not answer the
probe. That happens even on a cluster that has never sent a `txn`.
Evidence: the gate starts with nothing confirmed in each process, and only
`check()` (from `txn`) or `confirm()` fills it. Nothing calls `txn` after this
PR, so on an existing deployment `changePeerList` and `updatePdRaft` probe
every kept peer. Here are two cases jraft accepts today that this head refuses
with `TXN_NOT_PROPOSED`:
- 5 PD nodes, E is rebooting, C is dead. Replacing C with F as `A,B,D,E,F`:
E does not answer within `rpcTimeout`, so the change is refused. jraft would
commit it with the majority A, B, D, F.
- A rolling upgrade where A (leader) is upgraded and B is not yet, and C
dies. Replacing C with D as `A,B,D` is refused because B answers `maxKvOp` 0,
even though B is not joining.
Probing the kept peers buys no safety. A peer already in the current
configuration is checked by `check()` on every `txn`, because
`otherPeerEndpoints()` lists it, so an old or silent member already blocks
TXNs. The join-time check is only needed for peers that are not in the
configuration yet: they replay the log, and checking them closes the
check-then-propose window.
`testMembershipChangeProbesTheNewConfigurationAtOnce` asserts the refusal for a
silent peer, but never for one that is already a member.
Please pass `confirm` only the peers of `next` that are not in
`raftNode.listPeers()` / `listLearners()`. Please also add a test where a kept
member that is unreachable, or answers as an older release, does not block
`changePeerList`. The api-reference paragraph ("probes every peer of the new
configuration") should then say "every peer it adds".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
