Skip to content

[ISSUE #7401] Start heartbeat task in failure isolation test - #7402

Merged
Aias00 merged 1 commit into
apache:masterfrom
sunnysabor:fix/7401-heartbeat-test-start
Oct 2, 2026
Merged

Aias00 merged 1 commit into
apache:masterfrom
sunnysabor:fix/7401-heartbeat-test-start

Conversation

@sunnysabor

Copy link
Copy Markdown
Contributor

Fixes #7401.

HeartbeatFailureIsolationTest reads a scheduled task from the executor queue, but the test never started the subscriber, so no periodic task was scheduled. Start the subscriber before accessing the queue; the test can then invoke the scheduled task manually and verify that a failing heartbeat does not suppress other URIs or subsequent ticks.

Tests: ./mvnw -B -pl shenyu-client/shenyu-client-core -Djacoco.skip=true -Dmaven.javadoc.skip=true test (101 tests passed; Checkstyle clean).

@sunnysabor

Copy link
Copy Markdown
Contributor Author

CI follow-up: all triggered checks now pass, including the Kubernetes Gateway API integration job. The configured skips are unchanged. The focused shenyu-client-core run also passes all 101 tests with Checkstyle clean; this PR is ready for review.

@Aias00 Aias00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct fix for #7401. ShenyuClientURIExecutorSubscriber.start() — not the constructor — is what enqueues the periodic heartbeat task, and it is guarded by started.compareAndSet(false, true). Without this call executor.getQueue() is empty, so executor.getQueue().iterator().next() at line 57 throws NoSuchElementException. Calling start() after the URIs are registered is the natural placement.

The rest of the test is sound: the task is only ever executed manually (queue.remove(task); task.run();), assertFalse(task.isDone(), ...) is the real regression assertion for #7401, and executor.shutdownNow() in the finally keeps the heartbeat thread from leaking.

Nit: #7403 is the same one-line change, placed just after construction instead. Please coordinate and close one of the two.

@Aias00
Aias00 merged commit 39bcd00 into apache:master Oct 2, 2026
21 checks passed
@Aias00 Aias00 added this to the 2.7.2 milestone Nov 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Heartbeat failure isolation test reads an unscheduled task

2 participants