Copilot commented on code in PR #11:
URL: https://github.com/apache/shenyu-client-java/pull/11#discussion_r3990166447
##########
shenyu-client-core/src/main/java/org/apache/shenyu/client/core/dto/URIRegisterDTO.java:
##########
@@ -59,7 +61,7 @@ public class URIRegisterDTO implements DataTypeParent {
*/
public URIRegisterDTO(final String protocol, final String appName, final
String contextPath,
final String rpcType, final String host, final
Integer port,
- final EventType eventType, final String namespaceId)
{
+ final EventType eventType, final String namespaceId,
final String instanceInfo) {
Review Comment:
This changes the existing public eight-argument constructor instead of
preserving it. Downstream clients that call or were compiled against that
constructor will fail after upgrading; keep the old overload and delegate it to
the new constructor with a null instanceInfo value.
##########
shenyu-client-core/src/main/java/org/apache/shenyu/client/core/utils/SystemInfoUtils.java:
##########
@@ -17,31 +17,74 @@
package org.apache.shenyu.client.core.utils;
+import com.sun.management.OperatingSystemMXBean;
import java.lang.management.ManagementFactory;
Review Comment:
`ManagementFactory.getOperatingSystemMXBean()` returns the standard
`java.lang.management.OperatingSystemMXBean`; its implementation is not
required to implement the `com.sun.management` subtype on every Java 8 JVM.
This cast can therefore make `getSystemInfo()` fail, and the bootstrap
heartbeat task can stop permanently when that exception escapes its scheduled
callback. Only methods from the standard interface are used here, so use that
interface without the vendor-specific import/cast.
This issue also appears on line 74 of the same file.
##########
shenyu-client-core/src/main/java/org/apache/shenyu/client/core/disruptor/subcriber/ShenyuClientURIExecutorSubscriber.java:
##########
@@ -104,24 +107,25 @@ public void executor(final Collection<URIRegisterDTO>
dataList) {
}
ShenyuClientShutdownHook.delayOtherHooks();
shenyuClientRegisterRepository.persistURI(uriRegisterDTO);
-
+
URIS.add(uriRegisterDTO);
-
+
ShutdownHookManager.get().addShutdownHook(new Thread(() -> {
final URIRegisterDTO offlineDTO = new URIRegisterDTO();
BeanUtils.copyProperties(uriRegisterDTO, offlineDTO);
offlineDTO.setEventType(EventType.OFFLINE);
shenyuClientRegisterRepository.offline(offlineDTO);
-
+
// shutdown heartbeat executor
if (!executor.isTerminated()) {
executor.shutdown();
}
}), 2);
}
}
-
+
private void sendHeartbeat(final URIRegisterDTO uriRegisterDTO) {
+ uriRegisterDTO.setInstanceInfo(SystemInfoUtils.getSystemInfo());
Review Comment:
This probes host-wide hardware information once for every URI in `URIS`
every 30 seconds. With multiple registrations, the single heartbeat thread
repeats the OSHI construction, memory probe, and JSON serialization
unnecessarily and can spend increasing time on metadata rather than sending
heartbeats; compute the value once per cycle and reuse it for all DTOs (or
cache it).
--
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]
