qiuyanjun888 commented on code in PR #18416:
URL:
https://github.com/apache/dolphinscheduler/pull/18416#discussion_r3996259686
##########
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:
I checked this against the session lifecycle. updateById returning false
means the heartbeat row no longer exists, and the normal purge path removes
that row only after its stored session timeout has elapsed. The session has
therefore already expired; waiting for another local timeout would keep a stale
server active and reintroduce the split-brain window this PR fixes. I kept the
immediate DISCONNECTED transition, with the STARTED zero-row regression test
covering this case.
##########
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:
Addressed in fddfbe1a81. close() now atomically changes the server to
STOPPED only once, and subsequent concurrent or repeated calls return without
repeating scheduler shutdown or database cleanup. I also added a
latch-controlled concurrent regression test that verifies the client purge runs
only once.
##########
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:
Addressed in fddfbe1a81. The state field is now volatile and heartbeat
transitions use AtomicReferenceFieldUpdater.compareAndSet, so state reads and
transition methods no longer require synchronized. Connection callbacks run
only after the corresponding CAS succeeds. The small start/close lifecycle
critical section is retained to prevent startup and shutdown from overlapping.
--
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]
