ruanwenjun commented on code in PR #18416:
URL:
https://github.com/apache/dolphinscheduler/pull/18416#discussion_r3910027280
##########
dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-jdbc/src/main/java/org/apache/dolphinscheduler/plugin/registry/jdbc/server/JdbcRegistryServer.java:
##########
@@ -256,7 +256,9 @@ public void releaseJdbcRegistryLock(Long clientId, String
lockKey) {
@Override
public void close() {
- jdbcRegistryServerState = JdbcRegistryServerState.STOPPED;
+ synchronized (this) {
+ jdbcRegistryServerState = JdbcRegistryServerState.STOPPED;
+ }
Review Comment:
```suggestion
synchronized (this) {
if(jdbcRegistryServerState == JdbcRegistryServerState.STOPPED) {
log.warn("xx is already closed");
return
}
jdbcRegistryServerState = JdbcRegistryServerState.STOPPED;
}
```
##########
dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-jdbc/src/main/java/org/apache/dolphinscheduler/plugin/registry/jdbc/server/JdbcRegistryServer.java:
##########
@@ -341,27 +346,27 @@ private void refreshClientsHeartbeat() {
}
JdbcRegistryClientHeartbeatDTO clone =
jdbcRegistryClientHeartbeatDTO.clone();
clone.setLastHeartbeatTime(now);
-
jdbcRegistryClientRepository.updateById(jdbcRegistryClientHeartbeatDTO);
+ if (!jdbcRegistryClientRepository.updateById(clone)) {
+ log.error("The client heartbeat has expired: {}",
jdbcRegistryClientHeartbeatDTO.getId());
+ transitionToDisconnected();
+ return;
Review Comment:
```suggestion
throw xxException()
```
##########
dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-jdbc/src/main/java/org/apache/dolphinscheduler/plugin/registry/jdbc/server/JdbcRegistryServer.java:
##########
@@ -370,6 +375,30 @@ private void refreshClientsHeartbeat() {
}
}
+ private synchronized void transitionToStarted() {
+ if (jdbcRegistryServerState != JdbcRegistryServerState.SUSPENDED) {
+ return;
+ }
+ jdbcRegistryServerState = JdbcRegistryServerState.STARTED;
+ doTriggerReconnectedListener();
+ }
+
+ private synchronized void transitionToSuspended() {
+ if (jdbcRegistryServerState != JdbcRegistryServerState.STARTED) {
+ return;
+ }
+ jdbcRegistryServerState = JdbcRegistryServerState.SUSPENDED;
+ }
+
+ private synchronized void transitionToDisconnected() {
+ if (jdbcRegistryServerState != JdbcRegistryServerState.STARTED
+ && jdbcRegistryServerState !=
JdbcRegistryServerState.SUSPENDED) {
+ return;
+ }
+ jdbcRegistryServerState = JdbcRegistryServerState.DISCONNECTED;
+ doTriggerOnDisConnectedListener();
+ }
Review Comment:
Can we use `compareAndSwap` and `volatile` to reduce `synchronized`?
##########
dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-jdbc/src/main/java/org/apache/dolphinscheduler/plugin/registry/jdbc/server/JdbcRegistryServer.java:
##########
@@ -256,7 +256,9 @@ public void releaseJdbcRegistryLock(Long clientId, String
lockKey) {
@Override
public void close() {
- jdbcRegistryServerState = JdbcRegistryServerState.STOPPED;
+ synchronized (this) {
+ jdbcRegistryServerState = JdbcRegistryServerState.STOPPED;
+ }
Review Comment:
Avoid duplicate `close`?
--
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]
