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

Disable pantsd in inner runs. #7884

Conversation

@blorente
Copy link
Contributor

commented Jun 13, 2019

Problem

When a pants run under pantsd calls pants, it will also enable pantsd in the inner run. Since we don't allow parallel runs under pantsd, the inner run will block waiting for the outer run to complete, eventually timing out and crashing the whole run.
See #7881 for context.
The only real instance we've seen of this was entirely accidental.

Solution

Explicitly disable pantsd through an environment variable whenever we are serving a pailgun request.

Result

Inner runs under pantsd have the daemon turned off by default.
Fixes #7881 , Depends on #7890

Commits should be independently reviewable.

@blorente blorente requested review from illicitonion, stuhood, Eric-Arellano and ity and removed request for illicitonion Jun 13, 2019

@Eric-Arellano
Copy link
Contributor

left a comment

Good approach of TDD.

@blorente

This comment has been minimized.

Copy link
Contributor Author

commented Jun 18, 2019

Removing the needs-cherrypick flag, because the only cases where we see this behaviour already are implementing this manually.

@illicitonion
Copy link
Contributor

left a comment

I feel slightly squeamish about allowing this, as nested pants runs are generally an antipattern imo, but... It solves a need, I guess...

@ity
ity approved these changes Jun 18, 2019

@blorente blorente force-pushed the blorente:blorente/7881/disable-pantsd-in-inner-runs branch 4 times, most recently from f352c6b to 255339e Jun 21, 2019

@blorente blorente force-pushed the blorente:blorente/7881/disable-pantsd-in-inner-runs branch from 0d224fb to f6c7a25 Sep 4, 2019

@blorente

This comment has been minimized.

Copy link
Contributor Author

commented Sep 4, 2019

I rebased, and will merge if green. We unfortunately have seen this case in the wild once again.

@blorente blorente merged commit 612b5d6 into pantsbuild:master Sep 4, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
blorente added a commit that referenced this pull request Sep 4, 2019
Disable pantsd in inner runs. (#7884)
**Problem**
When a pants run under pantsd calls pants, it will also enable pantsd in the inner run. Since we don't allow parallel runs under pantsd, the inner run will block waiting for the outer run to complete, eventually timing out and crashing the whole run.
See #7881 for context.
The only real instance we've seen of this was entirely accidental.

**Solution**
Explicitly disable pantsd through an environment variable whenever we are serving a pailgun request.

**Result**
Inner runs under pantsd have the daemon turned off by default.
Fixes #7881 , Depends on #7890

Commits should be independently reviewable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.