Skip to content

/ saltfs

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

Make sccache never time out #763

Merged
merged 1 commit into from Dec 20, 2017
Merged

Make sccache never time out #763

merged 1 commit into from Dec 20, 2017

Conversation

@jdm
Copy link
Member

jdm commented Dec 14, 2017 •
edited by larsbergstrom

This should fix servo/servo#19495.

This change is Reviewable
@aneeshusa
Copy link
Member

aneeshusa commented Dec 14, 2017

Are there docs for this somewhere? Please add a link to the commit message and also maybe a description of the problem (where is sccache timing out currently?) and/or link to a Buildbot log in this PR.
@jdm
Make sccache never time out.
Loading status checks…
bbf08ca 
Per https://github.com/mozilla/sccache/blob/a310f6a62eb34281bda083f88dc96c736a3eeb74/src/server.rs#L69-L76 this variable controls how long
the server waits before shutting down. For long compiles that exceed this timeout, this can cause compilation to fail like
servo/servo#19495. https://github.com/mozilla/sccache/blob/629813f2d2f0b3ac8102696df69d57200688d320/docs/Jenkins.md#L11
describes the effect of making this timeout zero for CI infrastructure - the server process becomes long-lived and never shuts down.
@jdm jdm force-pushed the jdm-patch-16 branch from 3dc2d3b to bbf08ca Dec 14, 2017
@jdm
Copy link
Member Author

jdm commented Dec 14, 2017

Commit message updated.
@aneeshusa
Copy link
Member

aneeshusa commented Dec 14, 2017

Thanks for the links. Two questions I have:

  • The Jenkins.md link recommends start the server beside Jenkins as a system service in addition to setting the environment variable in this PR. Do you know if this is required/what the effect is if we don't start sccache by ourselves as a system service (e.g. upstart or systemd)?
  • We will need to upgrade sccache from time to time (e.g. #755). What is the story for handling these kinds of upgrades: do we need to perform manual steps, and if so, what are they? (Stopping the existing sccache process(es), starting new one(s), clearing cache data from disk, etc.)
@jdm
Copy link
Member Author

jdm commented Dec 19, 2017

The first time a build is run with a timeout of 0, it will automatically start the long-lived server in the background. As part of upgrading, we should run sccache --stop-server before installing the new version. Otherwise we should not need to do anything else. This PR should be mergeable as-is.
@jdm
Copy link
Member Author

jdm commented Dec 20, 2017

I'm going to go ahead and merge this change because servo/servo#19495 is really frustrating and I want to have enough time to be able to look into anything that might go wrong.

@bors-servo: r+
@bors-servo
Copy link
Contributor

bors-servo commented Dec 20, 2017

📌 Commit bbf08ca has been approved by jdm
@highfive highfive assigned jdm and unassigned aneeshusa Dec 20, 2017
@bors-servo
Copy link
Contributor

bors-servo commented Dec 20, 2017

⌛ Testing commit bbf08ca with merge 36dbe2c...
bors-servo added a commit that referenced this pull request Dec 20, 2017
@bors-servo
Auto merge of #763 - servo:jdm-patch-16, r=jdm
Loading status checks…
36dbe2c 
Make sccache never time out

This should fix servo/servo#19495.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/763)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Dec 20, 2017

☀️ Test successful - status-travis
Approved by: jdm
Pushing 36dbe2c to master...
@bors-servo bors-servo merged commit bbf08ca into master Dec 20, 2017
1 of 3 checks passed
1 of 3 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
homu Test successful
Details
@jdm jdm removed the S-needs-deploy label Dec 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jdm jdm

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Intermittent "Connection to server timed out" while building gecko-media (sccache)
4 participants
@jdm @aneeshusa @bors-servo @highfive
You can’t perform that action at this time.