Skip to content

Call PauseHandler resume from the resume endpoint - #1749

Open
Akhil-1527 wants to merge 2 commits into
spring-cloud:5.0.xfrom
Akhil-1527:fix-resume-after-pause
Open

Akhil-1527 wants to merge 2 commits into
spring-cloud:5.0.xfrom
Akhil-1527:fix-resume-after-pause

Conversation

@Akhil-1527

Copy link
Copy Markdown

/actuator/resume never calls resume() on the PauseHandler beans.

ResumeEndpoint.resume() only calls doResume() when the context is not running. That made sense when pause stopped the context, but since 7287a3c the pause endpoint calls the PauseHandler beans and the context keeps running. So after /actuator/pause, /actuator/resume returns false and does nothing, and anything paused through IntegrationPauseHandler stays paused.

This change tracks the paused state in RestartEndpoint and resumes when it is set. Resume still returns false when nothing was paused.

I added a test in RestartIntegrationTests that pauses and resumes with a test PauseHandler (it fails without the fix), and updated the docs line for these two endpoints, which still described the old Lifecycle behavior.

I ran into this while adding a Eureka PauseHandler for spring-cloud/spring-cloud-netflix#3841, which needs /actuator/resume to work.

The resume endpoint only called doResume() when the context was not
running. The pause endpoint no longer stops the context, so
/actuator/resume never reached the PauseHandler beans after
/actuator/pause. Track the paused state and resume when it is set.

Signed-off-by: Akhil CH <200716675+Akhil-1527@users.noreply.github.com>
@kdelay

kdelay commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

After pause then /actuator/restart, doRestart() keeps paused = true, and pauseHandlers still holds the beans of the closed context (it is only collected on the first refresh event).

Checked on this branch with a test PauseHandler: pause, doRestart(), resume returns true and calls resume() on the old context's handler only. Before this change it returned false there.

The stale list predates this PR, but re-collecting pauseHandlers and resetting paused in doRestart() would cover both.

@ryanjbaxter

Copy link
Copy Markdown
Contributor

As @kdelay points out we should handle the pause then restart case properly

After /actuator/pause then /actuator/restart the endpoint kept
paused = true and still held the PauseHandler beans of the closed
context. Clear both when the context closes and collect the
handlers again from the new context.

Signed-off-by: Akhil CH <200716675+Akhil-1527@users.noreply.github.com>
@Akhil-1527

Copy link
Copy Markdown
Author

Thanks @kdelay, good catch. I pushed a commit for it: doRestart() now clears the paused flag and the handler list once the old context is closed, and collects the PauseHandler beans again from the new context. testRestartAfterPause covers it: after pause and restart, resume returns false, and a new pause and resume reach the handler of the new context.

for (PauseHandler handler : this.pauseHandlers) {
handler.pause();
}
this.paused = true;

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.

I think we should only set this to true if !this.pauseHandlers.isEmpty(). Otherwise /actuator/resume will return true when nothing was paused

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants