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

Synchronize Job Coordination #319

Merged
merged 2 commits into from Jul 20, 2016
Merged

Synchronize Job Coordination #319

merged 2 commits into from Jul 20, 2016

Conversation

tgianos
Copy link
Contributor

@tgianos tgianos commented Jul 20, 2016

This PR synchronizes the entire Job Coordination code to make sure only one job per node can enter that block at a time and all metrics/job counts are consistent before another job is allowed.

This may slow down performance somewhat but the consistency tradeoff should be worth it. Will observe performance change in testing under load.

…time can be scheduled to be launched and we don't blow past the number of running jobs allowed on a node
@tgianos tgianos added this to the 3.0.0 milestone Jul 20, 2016
@tgianos tgianos self-assigned this Jul 20, 2016
@tgianos
Copy link
Contributor Author

tgianos commented Jul 20, 2016

@ajoymajumdar if you have time give this a quick peer review to see if you see any potential issues.

@coveralls
Copy link

coveralls commented Jul 20, 2016

Coverage Status

Changes Unknown when pulling bc0076e on 503Test into * on develop*.

@tgianos tgianos merged commit 0e23400 into develop Jul 20, 2016
@tgianos tgianos deleted the 503Test branch July 20, 2016 04:16
@@ -49,7 +49,7 @@ public JobCountServiceImpl(@NotNull final JobSearchService jobSearchService, @No
* {@inheritDoc}
*/
@Override
public int getNumJobs() {
public synchronized int getNumJobs() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need to synchronize this method?

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

3 participants