[go: nahoru, domu]

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CRaC: ignore checkpointOnRefresh afterRestore #32978

Closed
frederikz opened this issue Jun 7, 2024 · 4 comments
Closed

CRaC: ignore checkpointOnRefresh afterRestore #32978

frederikz opened this issue Jun 7, 2024 · 4 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@frederikz
Copy link

You do a spring.context.checkpoint=onRefresh. You do a CRaC restore. Your restored application needs to adapt its configuration which leads to a context refresh event which results in another checkpoint beeing created and the JVM exits.

Shouldn't checkpointOnRefresh be set to false in DefaultLifecycleProcessor in afterRestore (or after it was executed) or some other mechanism implemented that prevents the checkpointOnRefresh mechanism to be executed on a restored JVM? To me spring.context.checkpoint is an easy way to create a checkpoint and should never be executed on a restored JVM.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 7, 2024
@sdeleuze sdeleuze self-assigned this Jun 7, 2024
@sdeleuze sdeleuze added the in: core Issues in core modules (aop, beans, core, context, expression) label Jun 7, 2024
@sdeleuze
Copy link
Contributor
sdeleuze commented Jun 7, 2024

After the restoration, the execution is expected to resume after the point where checkpointOnRefresh is evaluated so I believe the current behavior is correct, and we have multiple tests checking that the behavior is correct on https://github.com/spring-projects/spring-lifecycle-smoke-tests.

Do you observe a concrete issue (if so, please provide a reproducer)?

@sdeleuze sdeleuze added the status: waiting-for-feedback We need additional information before we can continue label Jun 7, 2024
@frederikz
Copy link
Author

Yes, the after restore behavior is correct. But is a context refresh event an event that only occurs once in the lifetime of an application or is a context refresh allowed to happen at any time after an application is started? I think it is allowed to happen at any later time and this is e.g. how spring-cloud allows you to change the configuration at runtime with @RefreshScope. But when this happens a new context refresh event is sent and the onRefresh of DefaultLifecycleProcessor is called which then again triggers a CRaC checkpoint which is unwanted at this time.

I will try to create a small project which reproduces the issue.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jun 7, 2024
@sdeleuze sdeleuze added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jun 7, 2024
@frederikz
Copy link
Author

I thought the @RefreshScope from spring-cloud would lead to the second invocation of DefaultLifecycleProcessor.onRefresh but I was wrong and it was our own code were we in the ContextRefreshedEvent create a child ApplicationContext and refresh that. I have changed the code of https://github.com/frederikz/spring-boot-crac-demo to demonstrate similar behaviour. I don't know if the way we do it is to blame or if there could be more legitimate use cases that could lead to a second invocation of DefaultLifecycleProcessor.onRefresh which then would take a second CRaC checkpoint after restore.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jun 7, 2024
@sdeleuze sdeleuze added this to the 6.1.9 milestone Jun 7, 2024
@sdeleuze sdeleuze added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Jun 7, 2024
@sdeleuze
Copy link
Contributor
sdeleuze commented Jun 7, 2024

I am not sure I would recommend the use case you show here with a post restoration application creation, but the change you ask seems to be conceptually in line with the behavior we want for that feature that should only happen after startup, so I will refine our implementation accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants