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

Remove old heartbeats before starting heartbeat thread #1488

Merged
merged 1 commit into from Jul 7, 2016
Merged

Remove old heartbeats before starting heartbeat thread #1488

merged 1 commit into from Jul 7, 2016

Conversation

fw42
Copy link
Member

@fw42 fw42 commented Jul 7, 2016

It seems like #1485 did not fix the issue fully. I'm still seeing some PruneDeadWorker exceptions in production, which leads me to believe that there is still a race condition. Sadly I'm not really sure what the race condition is though :-(

This PR should once and for all fix it, by explicitly removing old heartbeats (of the same worker id) before sending any new heartbeats (and before registering the worker).

@dylanahsmith @sirupsen

@coveralls
Copy link

coveralls commented Jul 7, 2016

Coverage Status

Coverage increased (+0.2%) to 83.345% when pulling 39ffc38 on Shopify:synchronously-remove-old-heartbeats into 0691ac6 on resque:master.

@sirupsen
Copy link
Member

sirupsen commented Jul 7, 2016

:( I don't have any other ideas on the race. This doesn't seem crazy regardless, so I'm ok with it.

@dylanahsmith
Copy link
Contributor

👍

@fw42
Copy link
Member Author

fw42 commented Jul 7, 2016

something is weird with travis CI.. same error as the other branch.. unrelated to the changes here

@fw42
Copy link
Member Author

fw42 commented Jul 7, 2016

confirmed in production that this solves the errors I was seeing

@fw42 fw42 merged commit 27aaa64 into resque:master Jul 7, 2016
@fw42 fw42 deleted the synchronously-remove-old-heartbeats branch July 7, 2016 21:50
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.

None yet

4 participants