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

Crash when exception during loading occurs #275

Merged
merged 1 commit into from
Oct 4, 2018
Merged

Conversation

alenkacz
Copy link
Contributor

@alenkacz alenkacz commented Oct 4, 2018

Right now we were swallowing up these exceptions.

JIRA issues: DCOS_OSS-4239

def load(context: Context): Application = {
val jobComponents = new JobComponents(context)

jobComponents.metricsModule.start(jobComponents.actorSystem)

Future {
jobComponents.schedulerService.run()
}(scala.concurrent.ExecutionContext.global)
}(scala.concurrent.ExecutionContext.global).onComplete {
Copy link
Collaborator

@timcharper timcharper Oct 4, 2018

Choose a reason for hiding this comment

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

Not related to your pull request, but I just realized how terrible this is. We're blocking one of the global execution contexts threads for the entire life of metronome, just so the scheduler service can have something to block?

Copy link
Contributor Author

@alenkacz alenkacz Oct 4, 2018

Choose a reason for hiding this comment

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

haha, good point, that's really not nice...

Copy link
Contributor

@kensipe kensipe left a comment

Choose a reason for hiding this comment

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

what tim said :)

@alenkacz alenkacz merged commit 3c7eda2 into master Oct 4, 2018
@meichstedt meichstedt deleted the av/exception-loading branch October 17, 2018 10:31
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.

3 participants