bitflicker64 commented on code in PR #3185:
URL: https://github.com/apache/hugegraph/pull/3185#discussion_r3957146829
##########
hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java:
##########
@@ -203,7 +230,130 @@ public void shutDown() {
}
public boolean isLeader() {
- return this.raftNode.isLeader(true);
+ Node node = this.raftNode;
+ return node != null && node.isLeader(true);
+ }
+
+ /**
+ * Whether this node currently sees a raft leader.
+ * <p>
+ * A follower only keeps its leader while heartbeats keep arriving inside
the election
+ * timeout, and a leader only keeps its role while it can reach a quorum.
Seeing a leader
+ * therefore means this node is part of a quorum from its own point of
view, which is the
+ * signal a readiness probe needs. Served from the state machine
callbacks, not from the
+ * raft node, so it never waits on the node lock.
+ */
+ public boolean hasLeader() {
+ return this.raftNode != null &&
this.stateMachine.getProbeView().seesLeader;
+ }
+
+ /**
+ * Take a view of the local raft state from the volatile copies the state
machine
+ * callbacks maintain, never from the raft node itself. During an election
jraft holds
+ * the node lock while it reconnects to peers, so a probe that read the
node stalled for
+ * the connect timeout instead of answering its 503 promptly. All fields
derive from one
+ * read of one immutable view, swapped in whole per callback, so they
cannot contradict
+ * each other.
+ * <p>
+ * The state reported is the last one a callback announced: leader,
follower, error or
+ * shutdown. jraft emits no callback for candidacy or leadership transfer,
so a candidate
+ * reads as a follower that sees no leader, which yields the same
not-ready answer. The
+ * view trails the node by whatever sits in the FSM queue ahead of the
announcement,
+ * which is the price of never waiting on the node lock.
+ */
+ public RaftStatus getRaftStatus() {
Review Comment:
Fixed. The PR body now names `getRaftStatus()` and the nested `RaftStatus`
view (`isReady()`, `getState()`, `isLocalLeader()`), drops the
`isReady()`/`getNodeState()` claim, lists the twelve `RaftEngineReadinessTest`
cases actually in the file at `eb6de8f`, and adds `StoreAPIReadyTest`.
The local-verification line was rewritten to only what was measured at
`eb6de8f`: `pd-core-test` 104/104 (2 skipped) and `test-start-hugegraph-pd.sh`
13/13 against a PD built from this tree. The `pd-rest-test` count came from an
earlier head, so it now points at CI instead. Mirror #220 carries the same body
with the sync line kept first.
##########
hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java:
##########
@@ -203,7 +230,130 @@ public void shutDown() {
}
public boolean isLeader() {
- return this.raftNode.isLeader(true);
+ Node node = this.raftNode;
+ return node != null && node.isLeader(true);
+ }
+
+ /**
+ * Whether this node currently sees a raft leader.
+ * <p>
+ * A follower only keeps its leader while heartbeats keep arriving inside
the election
+ * timeout, and a leader only keeps its role while it can reach a quorum.
Seeing a leader
+ * therefore means this node is part of a quorum from its own point of
view, which is the
+ * signal a readiness probe needs. Served from the state machine
callbacks, not from the
+ * raft node, so it never waits on the node lock.
+ */
+ public boolean hasLeader() {
+ return this.raftNode != null &&
this.stateMachine.getProbeView().seesLeader;
+ }
Review Comment:
Applied in 667b6fb, as suggested: `hasLeader()` is now
`getRaftStatus().isReady()`, with the javadoc rewritten to say it answers
readiness. Verified at `eb6de8f`: `mvn test -pl hugegraph-pd/hg-pd-test -am -P
pd-core-test` passes 104/104 (2 skipped). Reasoning is on the earlier thread.
##########
hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java:
##########
@@ -182,6 +196,19 @@ public List<ChannelHandler> backChannelHandlers() {
}
public void shutDown() {
+ if (this.alivePeersRefresher != null) {
+ this.alivePeersRefresher.shutdownNow();
+ try {
+ // shutdownNow only interrupts; a refresh already inside
+ // listAlivePeers could otherwise publish a positive count
+ // after the reset below.
+ this.alivePeersRefresher.awaitTermination(1, TimeUnit.SECONDS);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ }
Review Comment:
Applied in 667b6fb, as suggested: the wait stays, the comment states what it
actually buys, and an overrun logs a warning naming the gauge. The generation
token is left for when `shutDown()` gains a production caller. Verified at
`eb6de8f`: `pd-core-test` 104/104 (2 skipped). Reasoning is on the earlier
thread.
--
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]
