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

Send ServerEvent when server startup has finished #4861

Conversation

tonygermano
Copy link
Collaborator

@tonygermano tonygermano commented Nov 16, 2021

An event is created early in the startup process, but there is not an event sent when startup completes.

resolves #4870

@jonbartels
Copy link
Contributor

There is a LOT of value in this one-line PR.

  1. The logging of the begin and end events lets an MC user measure MC startup time. This is important to measure since restarting MC fast can help in crisis situations or in horizontal scaling
  2. Monitoring integrations can watch for the relevant server events and report when systems are online
  3. Use cases like [IDEA] On mirth engine startup allow channel deployment deploy order by channel tag. #4860 become easier to implement (or work around) since they can register a listener and watch for the startup event then manually orchestrate channel deployment
  4. MC itself reports when the server is starting - the Login process from the MC Client returns a specific error code for when MC is still warming up. Other Mirth apps follow this pattern too.

@cturczynskyj
Copy link
Collaborator

@jonbartels Maybe I'm not understanding all of your bullet points, but to me, only the first one (timing startup) is particularly meritorious. I say this because there is already an endpoint that returns the server status (GET api/server/status) and when it returns <int>0</int> then the server has started.

@tonygermano @jonbartels Would one/both of you mind creating an issue to go with this PR and detail the use case(s) for it?

@tonygermano
Copy link
Collaborator Author

tonygermano commented Nov 19, 2021

I created an issue with the first point (which I think is enough) and also the third point.

I agree that if you are polling (especially externally) to see if the server is ready, monitoring changes in the events is likely not the best solution (unless you are doing it at the database level, maybe?)

@jonbartels
Copy link
Contributor

For my 2nd and 3rd points - If MC fires an event then an event listener can be registered. This is then a "push" operation instead of a "poll" operation as with the /status API. Once this is pushed any arbitrary workflow can then be triggered to do work since the MC instance is online and active. My example use cases may have been overly specific. The broader goal is that if MC fires an event for "starting" it should also fire an event for "ready".

#4870 serves my use cases and examples. Tony did a good job writing that up.

@cturczynskyj
Copy link
Collaborator

Thanks for doing that 👍

Copy link
Contributor

@jonbartels jonbartels left a comment

Choose a reason for hiding this comment

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

This solves the use cases stated in #4861

@pladesma pladesma added RS-7259 Internal-Issue-Created An issue has been created in NextGen's internal issue tracker labels Nov 16, 2022
@JackieK5
Copy link
Collaborator

@tonygermano Thank you for your Pull Request. We are interested in considering your contribution but before it can be included in an official branch or release, we need to receive a signed copy of our Contributor License Agreement. Please review the Source Code Contribution page for more details and then sign and return the agreement as instructed. NOTE: since we are also looking at #4868, you can sign just one release form and list both pull requests if you want.

@twest-mirthconnect
Copy link
Contributor

Same for this one @tonygermano. We need the contributor's agreement signed this month for planning purposes for 4.3.

Thanks in advance

@pladesma pladesma changed the base branch from development to pr4861-server-event-startup-finished June 16, 2023 19:33
@pladesma pladesma merged commit ba350be into nextgenhealthcare:pr4861-server-event-startup-finished Jun 16, 2023
@ChristopherSchultz
Copy link
Contributor

I'm curious about this:

[...] there is already an endpoint that returns the server status (GET api/server/status) and when it returns <int>0</int> then the server has started.

How are you supposed to call a server status endpoint before the server has started?

I guess you can just poll the expected endpoint with a short timeout and keep trying until you get a "0" back, eh?

@jonbartels
Copy link
Contributor

@ChristopherSchultz

startWebServer();
configurationController.setStatus(ConfigurationController.STATUS_ENGINE_STARTING);
startEngine();

Shows the conditions. The MC webserver (so basic API calls) is started before the actual engine is started. Then plugins, deployments, alerts, and some other gubbins.

Startup isn't considered done until here

configurationController.setStatus(ConfigurationController.STATUS_OK);
printSplashScreen();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internal-Issue-Created An issue has been created in NextGen's internal issue tracker RS-7259 triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[IDEA] Dispatch to the event log when server startup is complete
7 participants