bitflicker64 commented on code in PR #3358:
URL: https://github.com/apache/hugegraph/pull/3358#discussion_r4306711184
##########
hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java:
##########
@@ -203,6 +203,9 @@ void stopLeaderCallbacks() {
@Override
public void onLeaderStart(final long term) {
+ if (isTerminal()) {
Review Comment:
Important: This guard breaks an existing PD core test.
`hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/raft/RaftEngineReadinessTest.java`
(`testAlivePeerCountStopsAfterALeaderErrorsOrShutsDown`, lines 215-221) calls
`onError`, then `onLeaderStart(6)`, and asserts that `refreshAlivePeerCount()`
reads the node again (`assertEquals(2, ...)`, then `verify(mockNode,
times(2)).listAlivePeers()`). With this early return, `leaderTerm` stays -1
after `onError`, so the count stays -1. That test is in `PDCoreSuiteTest`,
which the `pd-core-test` step in `pd-store-ci.yml` runs. On JDK 17, `mvn test
-pl hugegraph-pd/hg-pd-test -am -P pd-core-test` at 5c302791 fails with 150
tests, 1 failure:
`RaftEngineReadinessTest.testAlivePeerCountStopsAfterALeaderErrorsOrShutsDown:221
expected:<2> but was:<-1>`. The same test passes at merge base d505f6f0. The
PR body lists only `StoreAPIReadyTest` runs, and PD CI has not run on this head
yet (HugeGraph-Server CI is `action_required`), so not
hing has caught this.
Requested change: update that test to the new contract. After `onError`, a
queued `onLeaderStart(6)` must leave the count at -1 and must not touch the
node. That means `assertEquals(-1, ...)` at line 221 and `verify(mockNode,
times(1)).listAlivePeers()` at the end, with the 'once per leader phase'
comment changed to match. Or split it into an error case and a shutdown case
that each start from a fresh leader. Then run the `pd-core-test` profile, not
just `StoreAPIReadyTest`.
--
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]
