menghaoranss commented on a change in pull request #6187: URL: https://github.com/apache/shardingsphere/pull/6187#discussion_r449445890
########## File path: shardingsphere-control-panel/shardingsphere-orchestration/shardingsphere-orchestration-core/shardingsphere-orchestration-core-registrycenter/src/main/java/org/apache/shardingsphere/orchestration/core/registrycenter/instance/OrchestrationInstance.java ########## @@ -29,21 +28,30 @@ public final class OrchestrationInstance { private static final String DELIMITER = "@"; - - private static final OrchestrationInstance INSTANCE = new OrchestrationInstance(); + + private static OrchestrationInstance instance; private final String instanceId; - - private OrchestrationInstance() { - instanceId = IpUtils.getIp() + DELIMITER + ManagementFactory.getRuntimeMXBean().getName().split(DELIMITER)[0] + DELIMITER + UUID.randomUUID().toString(); + + public OrchestrationInstance(final int identifier) { + instanceId = IpUtils.getIp() + DELIMITER + identifier + DELIMITER + UUID.randomUUID().toString(); Review comment: > what we really want is a singleton instance with the property accessible everywhere. > probably we should keep the constructor as it is and provide a `getInstance()` method throwing `IllegalStateException` if instance object is not created > > I do not think making an assignment like `instance = this` is a good idea. > @menghaoranss thoughts? Agree with you, may be we can keep the no arguments constructor , and then add a new method to generate `instanceId`. Generate `instanceId` when `OrchestrationInstance` is initialized in `RegistryCenter`. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org