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

Feature/monitor heartbeat checking #200

Merged
merged 7 commits into from
Feb 15, 2016
Merged

Conversation

rob-c
Copy link
Member

@rob-c rob-c commented Feb 15, 2016

This branch attempts to pull out the heartbeat code from #182 as it's likely that this branch could be superseded by other code due to work performed elsewhere.

This PR includes the code to inform the user if a worker thread is working and has been performing the same task for > 3min.
This 3min is currently not user configurable and is hard-coded as 180.

This PR also includes some very minor changes to improve the shutdown of the Monitoring loop at the end of it's life and some minor changes relating to the construction/teardown of the thread-pool which effects the GangaUnitTests derived tests as well as elsewhere.

This has already been discussed for inclusion in 6.1.16.

@rob-c rob-c self-assigned this Feb 15, 2016
@rob-c rob-c added this to the 6.1.16 milestone Feb 15, 2016
@milliams
Copy link
Contributor

In principle I'm happy for this to be merged. I'll need to review the code first but then we should be good.

@rob-c
Copy link
Member Author

rob-c commented Feb 15, 2016

There's nothing here not already in #182 which has been in for a while, the code has been through many iterations already.
I'm just breaking #182 into code I am certain to keep vs code I would like to keep.

@egede
Copy link
Member

egede commented Feb 15, 2016

Is it easy to make the 180 s configurable? Just in case we figure out that it is too short and that users get flooded with fake positives.

@rob-c
Copy link
Member Author

rob-c commented Feb 15, 2016

@egede I'll get on that now before I push to merge the PR, we may keep it at 5min or higher for 6.1.16 while it's in testing

@milliams
Copy link
Contributor

I guess a config item in [PollThread] would make sense. We can keep this low for testing but I agree with @rob-c for making it longer for the release,

@rob-c
Copy link
Member Author

rob-c commented Feb 15, 2016

OK, should be configurable now, and set to 300s by default.

@rob-c
Copy link
Member Author

rob-c commented Feb 15, 2016

I think this should be fixed for merging?

@milliams
Copy link
Contributor

👍

rob-c added a commit that referenced this pull request Feb 15, 2016
@rob-c rob-c merged commit 7dc459e into develop Feb 15, 2016
@rob-c rob-c deleted the featue/monitorHeartbeatChecking branch February 15, 2016 16:25
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.

None yet

4 participants