imbajin commented on code in PR #2962:
URL: https://github.com/apache/hugegraph/pull/2962#discussion_r2936765656
##########
hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/PDService.java:
##########
@@ -1735,6 +1737,17 @@ public void updatePdRaft(Pdpb.UpdatePdRaftRequest
request,
node.changePeers(config, status -> {
if (status.isOk()) {
log.info("updatePdRaft, change peers success");
+ // Refresh IpAuthHandler so newly added peers are not
blocked
+ IpAuthHandler handler = IpAuthHandler.getInstance();
+ if (handler != null) {
Review Comment:
⚠️ The new tests cover `IpAuthHandler.refresh()` in isolation, but they do
not exercise the actual regression path here: `changePeers(...) -> callback ->
handler.refresh(...)`. Could we add one regression test through the peer-list
update entrypoint as well? That would lock in the real behavior this PR is
fixing and prevent a future refactor from accidentally dropping the callback
refresh while the current tests still stay green.
```text
Current coverage
+--------------------+ +-------------------+
| IpAuthHandlerTest | ---> | handler.refresh() |
+--------------------+ +-------------------+
Missing regression path
+------------------+ +----------------+ +-------------------+
| changePeerList() | ---> | changePeers() | ---> | handler.refresh() |
+------------------+ +----------------+ +-------------------+
```
A small integration-style test around `RaftEngine.changePeerList()` or
`PDService.updatePdRaft()` would probably be enough.
--
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]
