Skip to content

[ISSUE #7701] Fix scaling monitor scheduler cleanup on context close. - #7799

Open
ymy4171 wants to merge 1 commit into
apache:masterfrom
ymy4171:fix/7701-scheduler-shutdown
Open

ymy4171 wants to merge 1 commit into
apache:masterfrom
ymy4171:fix/7701-scheduler-shutdown

Conversation

@ymy4171

@ymy4171 ymy4171 commented Oct 18, 2026

Copy link
Copy Markdown

Fixes #7701.

Closing the Spring application context previously left the scaling monitor's executor running because TaskSchedulerManager creates its scheduler internally and its existing shutdown() method was not registered as a destruction callback.

Add @PreDestroy to the existing cleanup method so context shutdown cancels registered tasks and shuts down the scheduler. Add real Spring-context regression tests covering both an active monitor task and a monitor task cancelled before context close; both assert executor shutdown and thread termination.

Validation:

  • Both new regression cases failed without the callback and passed with it.
  • On the rebased branch: Maven Wrapper 3.6.3 / JDK 21, ./mvnw -pl shenyu-admin verify (Windows wrapper, using local Maven mirror settings): 1998 tests, 0 failures, 0 errors, 1 skipped.
  • Checkstyle, Apache RAT, and git diff --check passed.
  • The full reactor was not rerun after rebasing. An earlier full-reactor baseline at b53408861 failed in the unrelated ApolloDataServiceTest.testProxySelectorDataDeleted test.

Checklist:

  • I have read the contribution guidelines.
  • I have added regression tests for the change.
  • Full-repository ./mvnw clean install -Dmaven.javadoc.skip=true passed; module validation and the earlier baseline limitation are described above.

@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.

Approved.

Adding @PreDestroy to TaskSchedulerManager.shutdown() correctly releases the monitor scheduler and cancels in-flight tasks when the application context closes, fixing the cleanup gap described in #7701. The new TaskSchedulerManagerTest drives the full Spring lifecycle (AnnotationConfigApplicationContext close) and asserts the executor terminates.

Minor note: CI checks have not reported on this branch yet; please confirm the workflow passes before merging.

This branch has not been deployed

No deployments
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] Scaling monitor scheduler survives Spring application-context shutdown

2 participants