[Fix-18274][Registry] Handle expired JDBC heartbeat sessions

[qiuyanjun888]

Was this PR generated or assisted by AI?

YES. This pull request was assisted by Hermes Agent / OpenAI Codex for code changes, focused tests, review feedback analysis, and local verification. The scope and final submission were directed by the contributor.

Purpose of the pull request

Closes #18274.

When the JDBC registry database is unavailable longer than the session timeout, another server can purge the stale heartbeat row. After recovery, the still-running client previously ignored the zero-row heartbeat update and incorrectly treated the refresh as successful.

This is an independent alternative related to #18275. It addresses the outstanding technical concerns discussed there by not recreating or upserting an expired heartbeat, which could revive a failed-over identity. Instead, a missing heartbeat enters the existing disconnect state machine so the owning service can terminate.

Brief change log

• Treat a zero-row JDBC heartbeat update as an expired registry session.
• Stop heartbeat refresh work after the registry server reaches DISCONNECTED.
• Persist the current heartbeat timestamp and add focused state-machine regression tests.

Verify this pull request

This change added tests and can be verified as follows:

• ./mvnw clean -pl dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-jdbc -am -DskipITs -Dtest=JdbcRegistryServerTest,JdbcRegistryDataChangeListenerAdapterTest -Dsurefire.failIfNoSpecifiedTests=false test
• Result: 4 tests, 0 failures, 0 errors, and BUILD SUCCESS across the selected reactor.
• ./mvnw -pl dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-jdbc spotless:apply

Pull Request Notice

Pull Request Notice

This pull request contains no incompatible change.

[SbloodyS]

When updateById(clone) returns false, the new RegistryException is caught by the generic error handler. If the server is currently STARTED, the catch block only moves it to SUSPENDED; it does not immediately invoke onDisConnected().

This can happen after a long JVM pause or heartbeat scheduler starvation:

1. Another registry server considers the session expired and removes its heartbeat, ephemeral data, and locks.
2. The original server resumes while its local state is still STARTED.
3. updateById() returns false.
4. The original server enters SUSPENDED and remains active until at least the next heartbeat cycle.

A missing heartbeat row is definitive evidence that the session has expired, not a transient database error. Continuing to run after that point can overlap with a server that has already taken over, leaving a split-brain window.

Please handle this condition separately and transition directly to DISCONNECTED, triggering the disconnection callback exactly once.

The new test sets the server state to SUSPENDED and lastSuccessHeartbeat to zero, so it bypasses the problematic STARTED branch. Please also add a test verifying that a zero-row heartbeat update while STARTED causes an immediate disconnection.

[qiuyanjun888]

When updateById(clone) returns false, the new RegistryException is caught by the generic error handler. If the server is currently STARTED, the catch block only moves it to SUSPENDED; it does not immediately invoke onDisConnected().

This can happen after a long JVM pause or heartbeat scheduler starvation:

1. Another registry server considers the session expired and removes its heartbeat, ephemeral data, and locks.
2. The original server resumes while its local state is still STARTED.
3. updateById() returns false.
4. The original server enters SUSPENDED and remains active until at least the next heartbeat cycle.

A missing heartbeat row is definitive evidence that the session has expired, not a transient database error. Continuing to run after that point can overlap with a server that has already taken over, leaving a split-brain window.

Please handle this condition separately and transition directly to DISCONNECTED, triggering the disconnection callback exactly once.

The new test sets the server state to SUSPENDED and lastSuccessHeartbeat to zero, so it bypasses the problematic STARTED branch. Please also add a test verifying that a zero-row heartbeat update while STARTED causes an immediate disconnection.

Thanks for your suggestions! I have already implemented, can you please help review?

[SbloodyS]

Thanks for addressing the previous feedback. A zero-row heartbeat update now disconnects the server immediately, and the added test covers the STARTED path.

However, there is still a shutdown race:

1. refreshClientsHeartbeat() passes the initial state check while the server is STARTED.
2. close() concurrently changes the state to STOPPED and deletes the heartbeat rows.
3. The in-flight updateById() returns false.
4. This branch unconditionally overwrites STOPPED with DISCONNECTED and invokes onDisConnected() during a normal shutdown.

Previously, the exception handler inspected the current state, so STOPPED was left unchanged. Please make the transition to DISCONNECTED conditional/atomic so that close() cannot race with the heartbeat task and have its terminal state overwritten. A regression test coordinating an in-flight heartbeat update with close() would also be useful.

[qiuyanjun888]

Addressed the shutdown race described in #18416 (comment) in commit 8d9599e.

• The transition to DISCONNECTED is now conditional and synchronized with close(), so a completed STOPPED transition cannot be overwritten and the disconnection callback is not invoked when close wins.
• Added a latch-controlled regression test that holds an in-flight heartbeat update, completes close(), then returns a zero-row update result.

Validation:

• JdbcRegistryServerTest: 5 tests, 0 failures, 0 errors
• module spotless:check: BUILD SUCCESS

Could you please take another look?

[SbloodyS]

There is still a shutdown race in the other state transitions.

close() and transitionToDisconnected() are synchronized, but the successful heartbeat and exception paths still update jdbcRegistryServerState directly:

• SUSPENDED -> STARTED after a successful refresh
• STARTED -> SUSPENDED after a refresh exception

For example:

1. The heartbeat thread observes SUSPENDED.
2. close() changes the state to STOPPED.
3. The heartbeat thread writes STARTED and invokes onReconnected().

The exception path can similarly overwrite STOPPED with SUSPENDED. Since the field is also neither volatile nor consistently accessed under the same lock, state visibility is not guaranteed.

Please make all state transitions atomic and use one synchronization strategy for every read/write, such as synchronized transition methods or an AtomicReference with compare-and-set. Connection callbacks should only run when their corresponding transition succeeds.

Please also add regression coverage for close() racing with:

• A successful heartbeat while the server is SUSPENDED.
• A heartbeat exception while the server is STARTED.

In both cases, STOPPED must remain the final state and no reconnect/disconnect callback should be triggered after shutdown.