fix(store): keep GET /v1/partitions working on follower stores

[bitflicker64]

Purpose of the PR

• Fixes [Bug] Store GET /v1/partitions answers 500 "Not leader" on any Store that follows at least one raft group #3230

GET /v1/partitions on a Store returns 500 IllegalStateException: Not leader as soon as it reaches a raft group the Store follows. PartitionAPI.getPartitions calls PartitionEngine.getCurrentConf() for every group, and getCurrentConf() builds the Configuration from raftNode.listPeers() and raftNode.listLearners(). jraft's NodeImpl throws Not leader from both unless the node is the leader. With more than one replica per group every Store follows some groups, so the endpoint fails on every Store in the cluster.

Main Changes

• PartitionAPI.getPartitions: move raft.setConf(...) inside the engine.isLeader() block that already guards setPeers and setLearners. On followers conf is now null, like peers and learners; all other fields (role, leader, term, logIndex, partitions and metrics) are filled as before.
• PartitionEngine.getCurrentConf() itself is unchanged, since its other callers (changePeers, onLeaderStart, onStartFollowing) are outside this endpoint.
• TODO comments, no behavior change, at dead code found nearby: a dead return and an unused DashResponse line in PartitionAPI; a commented-out changeShards block, an unused groupId with an unlogged error in doBlankTaskSync, and a silent replicator onError in PartitionEngine; and HgStoreNodeServiceTest, whose @Test annotations are all commented out.
• New PartitionAPITest in hg-store-node/src/test, next to the existing ScanUtilTest. It mocks one follower group and one leader group. The follower's Node.listPeers(), Node.listLearners() and getCurrentConf() throw IllegalStateException("Not leader"), which is how jraft behaves. The test checks that both groups are listed, that the follower has conf, peers and learners set to null, and that the leader still reports its conf and peers.

Verifying these changes

• Trivial rework / code cleanup without any test coverage. (No Need)
• Already covered by existing tests, such as (please modify tests here).
• Need tests and can be verified as follows:

  • PartitionAPITest#testGetPartitionsOnFollower, run with JDK 11:

    mvn test -pl hugegraph-store/hg-store-node -am -Dtest=PartitionAPITest \
      -DfailIfNoTests=false -Dsurefire.failIfNoSpecifiedTests=false

    Without the PartitionAPI change it fails with the error from the issue:

    [ERROR]   PartitionAPITest.testGetPartitionsOnFollower:52 » IllegalState Not leader
    [ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0
    

    With the change, together with the other hg-store-node tests (-Dtest='PartitionAPITest,ScanUtilTest,JraftMetricsTest'):

    Tests run: 1, Failures: 0, Errors: 0, Skipped: 0 - in org.apache.hugegraph.store.node.metrics.JraftMetricsTest
    Tests run: 6, Failures: 0, Errors: 0, Skipped: 0 - in org.apache.hugegraph.store.node.grpc.ScanUtilTest
    Tests run: 1, Failures: 0, Errors: 0, Skipped: 0 - in org.apache.hugegraph.store.node.controller.PartitionAPITest
    Tests run: 8, Failures: 0, Errors: 0, Skipped: 0
    BUILD SUCCESS
    

  • mvn editorconfig:check -pl hugegraph-store/hg-store-node passes.

  • I have not yet re-run this against a live 3-Store cluster. The verification above is the unit test only.

Does this PR potentially affect the following parts?

• Dependencies (add/update license info & regenerate_known_dependencies.sh)
• Modify configurations
• The public API
• Other affects (typed here)
• Nope

Store REST GET /v1/partitions now returns 200 on a Store that follows some groups instead of 500. For those groups conf is null in the response. Nothing changes for groups the Store leads.

Documentation Status

• Doc - TODO: required documentation is pending; complete it before merging.
• Doc - Done: documentation is included here or linked below.
• Doc - No Need: no user-visible documentation is affected.

Documentation files in this PR or paired hugegraph-doc PR:

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 41.34%. Comparing base (83ef9f3) to head (fc017f8).

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3232      +/-   ##
============================================
- Coverage     41.35%   41.34%   -0.01%     
+ Complexity     7299     7295       -4     
============================================
  Files           802      802              
  Lines         69688    69688              
  Branches       9291     9291              
============================================
- Hits          28816    28812       -4     
- Misses        37576    37587      +11     
+ Partials       3296     3289       -7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[bitflicker64]

Blocking: no. Summary: Moving setConf inside the isLeader() block fixes the follower 500 from #3230, and the new PartitionAPITest reproduces the jraft contract and runs in the store CI job. One non-blocking note: the isLeader() check and the three leader-only calls are not atomic, so a leader step-down during the request can still return 500. Evidence: read PartitionAPI.getPartitions and PartitionEngine.getCurrentConf/isLeader at fc017f8; read NodeImpl.listPeers and isLeader(false) in jraft-core 1.3.13 sources; confirmed PartitionAPITest ran and passed in each hg-store-test step of the store job log; all workflows on this head passed, only codecov/project reports -0.01%.

[bitflicker64]

Minor: engine.isLeader() calls raftNode.isLeader(false), which reads state without the node lock. getCurrentConf(), listPeers() and listLearners() then each take the read lock and throw IllegalStateException("Not leader") if state is no longer STATE_LEADER (jraft-core 1.3.13 NodeImpl.listPeers). If the group steps down between the check and these calls, for example during a PD leader balance, the whole GET /v1/partitions response is still a 500. This race existed before for listPeers() and the PR does not make it worse, so it does not block.

If you want the endpoint to survive leader transfers too, wrap the three calls in one try { ... } catch (IllegalStateException e) { ... } and leave conf, peers and learners null on failure, the same as the follower case. A second follower-style mock whose isLeader() returns true while the node methods throw would cover it.