bitflicker64 commented on code in PR #3385:
URL: https://github.com/apache/hugegraph/pull/3385#discussion_r4330743458


##########
hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/raft/RaftEngineReadIndexTest.java:
##########
@@ -70,6 +70,17 @@ public class RaftEngineReadIndexTest {
     public void setUp() {
         RaftEngine engine = RaftEngine.getInstance();
         this.originalRaftNode = engine.getRaftNode();
+        RaftStateMachine machine = Whitebox.getInternalState(engine, 
"stateMachine");
+        this.leaderTerm = Whitebox.getInternalState(machine, "leaderTerm");
+        this.originalLeaderTerm = this.leaderTerm.get();
+        // The engine's alive-peers refresher calls listAlivePeers() on 
whatever node
+        // the singleton holds whenever this machine reports a leader, and a 
call from
+        // that thread while a stubbing runs here is what Mockito reports as an
+        // unfinished stubbing. Keep the machine not-leader while the mock is
+        // installed: the refresher then leaves the mock alone, and the tests 
below
+        // announce a term only once every stubbing has completed.
+        this.leaderTerm.set(-1L);

Review Comment:
   Important: `leaderTerm` at -1 keeps the refresher off this mock, but other 
threads still call it. `RaftEngine.isLeader()` calls `raftNode.isLeader(true)` 
without reading `leaderTerm`. `PDCoreTestBase.initService()` starts a 
`TaskScheduleService` for each of the 8 suite classes before this one, and 
`shutdownService()` never stops it, so their 1 s `clearTTLData` task (`if 
(isLeader())`) keeps calling `isLeader(true)` on this mock. A call from another 
thread between `doAnswer(...).when(this.mockNode)` and `.readIndex(...)` takes 
the pending answer, which gives the #3384 `UnfinishedStubbingException`. A 
probe placed just before this class in `PDCoreSuiteTest` saw 42 such calls in 5 
s from 8 executor pools.
   
   Please stub `readIndex` only once, in `setUp` before the mock is installed, 
with an answer that delegates to a `volatile Answer<Void>` field. The tests 
that now call `doAnswer`/`doThrow(...).when(this.mockNode)` would assign that 
field instead. The order of the `leaderTerm` writes and the setUp comment then 
no longer matter.



-- 
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]

Reply via email to