-
Notifications
You must be signed in to change notification settings - Fork 316
bugfix: PostBoot commands now run after server ready event, not durin… #7808
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
Conversation
…g service construction. This is due to the fact that previously, deploy would fail due to server not being ready for deployment fixes payara#7806
|
I'll need to find some time to review this carefully. I believe the state of the world is currently as such:
I also recently reordered when exactly that Done so here: #7576 In that PR I introduced a property as a fallback which should still be present, but that was intended as a "just in case" due to the delicate nature of this sequencing (butterflies and hurricanes): |
| @Override | ||
| public void event(Event<?> event) { | ||
| if (event.is(EventTypes.SERVER_READY)) { | ||
| doBootCommands(startupContext.getArguments().getProperty("-postbootcommandfile"), true); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void postConstruct() { | ||
| doBootCommands(startupContext.getArguments().getProperty("-postbootcommandfile"), true); | ||
| events.register(this); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my other comment regarding sequencing.
This will likely reintroduce race conditions and sequencing issues: the postboot config file would be executed after all but new applications in the deploy dir have been deployed. Tying this service to an event rather than a specific run level will also mean that it races against all other services reacting to that event.
|
Thanks for looking at this. The "Anchor issue" still remains unresolved (#7655) and any help there would be appreciated. |
…g service construction.
fixes #7806
This is due to the fact that previously, deploy would fail due to server not being ready for deployment fixes #7806
Description
Important Info
Blockers
Testing
Testing Performed
Tested CDI app in postboot