Copilot commented on code in PR #2961:
URL: https://github.com/apache/hugegraph/pull/2961#discussion_r2904689605
##########
hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java:
##########
@@ -239,8 +241,20 @@ public String getLeaderGrpcAddress() throws
ExecutionException, InterruptedExcep
waitingForLeader(10000);
}
- return
raftRpcClient.getGrpcAddress(raftNode.getLeaderId().getEndpoint().toString()).get()
- .getGrpcAddress();
+ try {
+ RaftRpcProcessor.GetMemberResponse response = raftRpcClient
+
.getGrpcAddress(raftNode.getLeaderId().getEndpoint().toString())
+ .get(config.getRpcTimeout(), TimeUnit.MILLISECONDS);
Review Comment:
`waitingForLeader(10000)` is called when `raftNode.getLeaderId()` is null,
but the result isn’t used/rechecked. If the leader still isn’t elected after
the wait, the subsequent `raftNode.getLeaderId().getEndpoint()` (and the
fallback path) will still throw an NPE. Capture the leader into a local
variable after waiting and handle the `null` case explicitly (e.g., throw a
clear exception or return a sentinel that callers can handle).
##########
hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java:
##########
@@ -239,8 +241,20 @@ public String getLeaderGrpcAddress() throws
ExecutionException, InterruptedExcep
waitingForLeader(10000);
}
- return
raftRpcClient.getGrpcAddress(raftNode.getLeaderId().getEndpoint().toString()).get()
- .getGrpcAddress();
+ try {
+ RaftRpcProcessor.GetMemberResponse response = raftRpcClient
+
.getGrpcAddress(raftNode.getLeaderId().getEndpoint().toString())
+ .get(config.getRpcTimeout(), TimeUnit.MILLISECONDS);
+ if (response != null && response.getGrpcAddress() != null) {
+ return response.getGrpcAddress();
+ }
+ } catch (TimeoutException | ExecutionException e) {
+ log.warn("Failed to get leader gRPC address via RPC, falling back
to endpoint derivation", e);
Review Comment:
This `log.warn(..., e)` will likely be hit on every follower redirect in
environments where the RPC path is consistently broken/blocked (the scenario
described in #2959), which can flood logs and add overhead due to repeated
stack traces. Consider reducing verbosity (e.g., warn without stack trace,
debug with stack trace, or rate-limit) and include key context like leader
endpoint/timeout in the message.
```suggestion
String leaderEndpoint = raftNode.getLeaderId() != null
?
String.valueOf(raftNode.getLeaderId().getEndpoint())
: "unknown";
long timeoutMs = config.getRpcTimeout();
log.warn(
"Failed to get leader gRPC address via RPC, falling back
to endpoint derivation: " +
"leaderEndpoint={}, timeoutMs={}, errorType={},
errorMessage={}",
leaderEndpoint, timeoutMs, e.getClass().getSimpleName(),
e.getMessage());
log.debug("Stack trace for failed getLeaderGrpcAddress RPC", e);
```
--
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]
