Aias00 commented on code in PR #7762:
URL: https://github.com/apache/shenyu/pull/7762#discussion_r4303359918
##########
shenyu-register-center/shenyu-register-client-beat/src/main/java/org/apache/shenyu/register/client/beat/HeartbeatListener.java:
##########
@@ -113,29 +166,124 @@ public HeartbeatListener(final
ShenyuBootstrapHeartBeatConfig config, final Shen
private void sendHeartbeat(final InstanceBeatInfoDTO instanceBeatInfoDTO) {
for (String server : serverList) {
- String concat = server.concat(Constants.BEAT_URI_PATH);
- try {
- String accessToken = this.accessToken.get(server);
- if (StringUtils.isBlank(accessToken)) {
- throw new NullPointerException("accessToken is null");
+ AtomicBoolean serverInFlight = null;
+ if (!directExecution) {
+ serverInFlight = inFlight.computeIfAbsent(server, k -> new
AtomicBoolean(false));
+ if (!serverInFlight.compareAndSet(false, true)) {
+ LOG.warn("Previous heartbeat to admin url :{} is still in
flight, skipping.", server);
+ continue;
+ }
+ }
+ final AtomicBoolean finalInFlight = serverInFlight;
+ Future<?> future = beatExecutor.submit(() -> {
Review Comment:
The flag is cleared only inside the runnable (`finally`), so any path where
this task never runs leaves `inFlight` stuck `true` and that admin gets no
further heartbeats until restart. Two such paths: (a) a queued task cancelled
by `future.cancel(true)` at the deadline before it starts, (b) a
`DiscardPolicy` rejection, where `submit` still returns a never-completing
`FutureTask`. Clearing the flag in the timeout branch too (idempotent) or using
`CompletableFuture.runAsync(...).whenComplete(...)` closes both. Details in the
review body.
##########
shenyu-register-center/shenyu-register-client-beat/src/main/java/org/apache/shenyu/register/client/beat/HeartbeatListener.java:
##########
@@ -84,14 +120,31 @@ public HeartbeatListener(final
ShenyuBootstrapHeartBeatConfig config, final Shen
this.password = AesUtils.cbcEncrypt(secretKey, secretIv, password);
}
this.serverList =
Lists.newArrayList(Splitter.on(",").split(config.getServerLists()));
+ String timeoutProp =
config.getProps().getProperty(HEARTBEAT_TIMEOUT_MS);
+ this.deadlineMs = StringUtils.isNotBlank(timeoutProp) ?
Long.parseLong(timeoutProp) : DEFAULT_DEADLINE_MS;
Review Comment:
`Long.parseLong` here throws `NumberFormatException` out of the constructor,
and this bean is created during startup (`HeartbeatListenerConfiguration:47`),
so a typo such as `heartbeatTimeoutMs=4s` prevents the gateway from booting.
Please fall back to `DEFAULT_DEADLINE_MS` with a warn instead.
--
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]
