bitflicker64 opened a new pull request, #3232:
URL: https://github.com/apache/hugegraph/pull/3232

   ## Purpose of the PR
   
   - Fixes #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.
   - 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)*.
   - [x] Need tests and can be verified as follows:
       - `PartitionAPITest#testGetPartitionsOnFollower`, run with JDK 11:
   
         ```bash
         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](https://hugegraph.apache.org/docs/contribution-guidelines/contribute/#321-check-licenses)
 info & 
[regenerate_known_dependencies.sh](../install-dist/scripts/dependency/regenerate_known_dependencies.sh))
   - [ ]  Modify configurations
   - [x]  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.
   - [x] `Doc - No Need`: no user-visible documentation is affected.
   
   Documentation files in this PR or paired hugegraph-doc PR:
   


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