SbloodyS commented on PR #18416: URL: https://github.com/apache/dolphinscheduler/pull/18416#issuecomment-4977797638
When [updateById(clone) returns false](https://github.com/apache/dolphinscheduler/blob/0abdcc6ae22bcf76b7f0ab00aff098e4ce2fb85f/dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-jdbc/src/main/java/org/apache/dolphinscheduler/plugin/registry/jdbc/server/JdbcRegistryServer.java#L344-L349), the new RegistryException is caught by the generic error handler below. If the server is currently STARTED, [the catch block only moves it to SUSPENDED](https://github.com/apache/dolphinscheduler/blob/0abdcc6ae22bcf76b7f0ab00aff098e4ce2fb85f/dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-jdbc/src/main/java/org/apache/dolphinscheduler/plugin/registry/jdbc/server/JdbcRegistryServer.java#L359-L370); it does not immediately invoke onDisConnected(). This can happen as follows: A long JVM pause or heartbeat scheduler starvation occurs while the local state remains STARTED. Another registry server considers the session expired and removes its heartbeat, ephemeral data, and locks. The original server resumes and updateById() returns false. The current implementation only enters SUSPENDED, so the expired server remains active until at least the next heartbeat cycle—and potentially until the full session timeout if the row was deleted shortly after the last successful heartbeat. A missing heartbeat row is definitive evidence that the session has expired, not an ordinary transient database error. Continuing to run after that point can overlap with the server that has taken over, creating the split-brain condition this change is intended to prevent. The related review discussion also explicitly states that an expired service should terminate to avoid split-brain behavior: [PR #18275 discussion](https://github.com/apache/dolphinscheduler/pull/18275#discussion_r3301672139). Please use a distinct session-expired exception or an explicit state-transition method that immediately moves the server to DISCONNECTED and triggers the disconnection callback exactly once. The new test preconfigures the server as SUSPENDED and sets lastSuccessHeartbeat to zero, so it bypasses the problematic STARTED branch. Please add a regression test verifying that a zero-row heartbeat update from STARTED causes an immediate disconnection. -- 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]
