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

[pantsd] Don't ignore the first watchman event for the daemon pid #8096

Merged
merged 2 commits into from Jul 24, 2019

Conversation

@blorente
Copy link
Contributor

commented Jul 23, 2019

Problem

The SchedulerService checks every event that changes the pantsd pidfile, and invalidates the daemon if the pid no longer is what it should be.

However, it also ignores the first event of every subscription. This is not something we want to do for the pidfile subscription, because if the pidfile changes between the time we write it and the time we subscribe to it and receive the first event, we will only receive the first event, and will continue on without invalidating the daemon.

This has happened in the following situation:

  • Two invocations of pants are triggered at the same time on a machine, both with pantsd, and pantsd was not active at the moment. Let's call them inv1 and inv2.
  • Both inv1 and inv2 see that there is not pidfile, and therefore they assume no pantsd is running, and thus start creating one each, in parallel.
  • inv1 finishes creating the services first and writes its pidfile.
  • inv2 finishes creating the services, and overwrites the pidfile.
  • inv1 registers the watchman subscritption for the pidfile. Watchman triggers the initial event for the pidfile in inv1. Inv1 ignores it, and continues running. It will never receive any request, because its pidfile is not there, but it will not invalidate until the pidfile is changed again.

Solution

Not ignoring the first event for the pidfile.

Result

More robust initialization of pantsds, where in the condition described above inv1 would check the pidfile and invalidate, leaving only inv2.

Commits are independently reviewable.

@blorente blorente requested review from stuhood and illicitonion Jul 23, 2019

@stuhood
Copy link
Member

left a comment

Thanks!

@blorente blorente merged commit e41ef3b into pantsbuild:master Jul 24, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.