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

Enable ccache for Alpine builds on Travis CI #918

Conversation

PeterDaveHello
Copy link
Member

@PeterDaveHello PeterDaveHello commented Nov 19, 2018

cc #786

@PeterDaveHello PeterDaveHello force-pushed the enable-ccache-for-TravisCI-alpine-builds branch 2 times, most recently from dc6d32d to 8003f10 Compare November 19, 2018 19:42
@PeterDaveHello PeterDaveHello requested review from chorrell, LaurentGoderre and a team and removed request for chorrell November 19, 2018 19:55
@PeterDaveHello PeterDaveHello changed the title Enable ccache for Alpine builds on Travis CI [WIP] Enable ccache for Alpine builds on Travis CI Nov 19, 2018
@PeterDaveHello PeterDaveHello force-pushed the enable-ccache-for-TravisCI-alpine-builds branch from 8003f10 to 481f3fd Compare November 19, 2018 23:16
@PeterDaveHello
Copy link
Member Author

PeterDaveHello commented Nov 20, 2018

The first build with ccache will take longer time than usual, and may exceed the hard limit 50 mins for free plan on Travis CI, no ideas on how to fix it yet.

@PeterDaveHello
Copy link
Member Author

The latest record we didn't skip any build: https://travis-ci.com/nodejs/docker-node/builds/91724465
Looks like newer versions take more time, and it's very close to the hard limit of the Travis CI free plan, so sooner or later, we need to deal with it, not sure if Enterprise plan would be an option?

cc @nodejs/docker

@PeterDaveHello
Copy link
Member Author

After a few retires, it passed now.

@PeterDaveHello PeterDaveHello force-pushed the enable-ccache-for-TravisCI-alpine-builds branch 2 times, most recently from 53a7b30 to c672865 Compare November 20, 2018 09:47
@PeterDaveHello
Copy link
Member Author

Wait for #919 to unlock CI blocking.

@chorrell
Copy link
Contributor

I approved and merged #919

@PeterDaveHello PeterDaveHello force-pushed the enable-ccache-for-TravisCI-alpine-builds branch 13 times, most recently from 294faa9 to e055d20 Compare November 21, 2018 10:37
@LaurentGoderre
Copy link
Member

Wow!! Didn't know about netcat!! Really cool!

@chorrell
Copy link
Contributor

Yeah!

This was a long journey by well worth it. A lot of groundwork covered by #786 too

@LaurentGoderre
Copy link
Member

The only thing I wonder is how will netcat react if nothing is listening to that port for the docker official builds?

@chorrell
Copy link
Contributor

Yeah, not sure.

@PeterDaveHello
Copy link
Member Author

Then it failed, it's fine, as the trailing || true will cover it from breaking the build.

@PeterDaveHello
Copy link
Member Author

PeterDaveHello commented Nov 21, 2018

Well, I missed the later || true, will send a PR to fix it.

@PeterDaveHello
Copy link
Member Author

PR sent: #923

@PeterDaveHello
Copy link
Member Author

PeterDaveHello commented Nov 21, 2018

Looks strange, the cache fetching process is not so reliable in real situation(however, it looks fine on my own environment, that's why it took me 2 days to test the cases), for example, in the latest master branch build: https://travis-ci.com/nodejs/docker-node/builds/92180608, v10 & v11 fetched and extracted the cache without problem, v6 & v8 failed, so if this keeps happening, I'm going to switch to use a simple python http server to handle it, which should be more reliable, I didn't do it because it looks not so pretty as the netcat method should work on both download and upload side usually, I don't want to use two different tools and methods to handle it, but looks like the most easiest way here is to use a workaround.

@PeterDaveHello
Copy link
Member Author

Oh nope, looks like this is only happenning on v6 image(ignore the v8 one as it's just updated), which is based on Alpine v3.4, may be another software issue, will need the update of v6 to fix it.

To be clear, my testing environment is on Ubuntu 16.04.5 host and Alpine v3.6 container, didn't recall that we have old to Alpine v3.4.

@PeterDaveHello
Copy link
Member Author

From the v8 build here as we can see: https://travis-ci.com/nodejs/docker-node/jobs/160105110, a version update will make the cache all missed, so we also need to have an mechanism to clear the cache once the version was changed in case the cache going so fat, and the most important, useless.

@PeterDaveHello
Copy link
Member Author

I can reproduce that issue now.

  • netcat-openbsd v1.89-r2 on Alpine v3.4/3.5 receives file with addition bytes, which make the cache tarball unable to be extracted(no problem with Alpine v3.6+, which use netcat-openbsd v1.130-r1+)
  • built-in busybox nc on Alpine v3.4~3.8 receives file properly on my testing environment, but doesn't close the connection problem properly, though we can use timeout command to handle that, but the same process's transferring results are wrong on our Travis CI, which means we can't use it in fact (reason unknown yet, hard to debug on Travis CI, maybe it's because the connection didn't close properly, the received file size is always 2KiB)

So this is the strange case I didn't noticed before, the workaround is to use other method like http transferring, but a little bit ugly, or, as it's just on v6 image, we can wait until it got any update(unpredictable?).

Summary: current method can't work on node.js v6 based on alpine v3.4, current workaround is ugly and will be applied to all Alpine images, which I don't want to, just waiting may be also an option.

Not sure if I explain well, let me know what do you guys think, thanks.

@LaurentGoderre
Copy link
Member

@PeterDaveHello i don't understand, a version change should not invalidate the cache

@chorrell
Copy link
Contributor

I think it's worth breaking our update policy in this case and update the v6 alpine image to 3.8. Alpine 3.4 dropped out of support earlier this year:

https://wiki.alpinelinux.org/wiki/Alpine_Linux:Releases

So having it on an unsupported Alpine version is probably worse from an LTS/security update perspective than the possible breaking change of going from 3.4 to 3.8 with a non-Node.js update. We might as well bump yarn too while we're at it.

@PeterDaveHello
Copy link
Member Author

@LaurentGoderre The ccache summary shows the updated version hit 0% cache, I'll have another test for it, but here is the result saw on our build log.

@LaurentGoderre
Copy link
Member

I'm surprised you don't have to extract the cache from the image. Too bad we can't inspect the cache in more details

@LaurentGoderre
Copy link
Member

@PeterDaveHello I can't get this to work for me locally. The build gets connection refused for nc

@LaurentGoderre
Copy link
Member

Hmmmm! we have to pass --network=host to docker build otherwise sometimes you get the bridge IP address and not the one that is listening

@LaurentGoderre
Copy link
Member

@PeterDaveHello btw, are you aware of Pipe Viewer (pv)? It's awesome for netcat!

@PeterDaveHello
Copy link
Member Author

Yes I know pv but it's no need when building Docker image, as its standard output won't appear on the screen

@PeterDaveHello
Copy link
Member Author

Directly tested on Ubuntu 16.04.5 amd64, build node-v8.12.0 and then node-v8.13.0, as you can see, no cache hit:

cache directory                     /home/peter/.ccache
primary config                      /home/peter/.ccache/ccache.conf
secondary config      (readonly)    /etc/ccache.conf
cache hit (direct)                     0
cache hit (preprocessed)               0
cache miss                          4564
called for link                       16
called for preprocessing               6
unsupported source language           52
no input file                          2
files in cache                     13766
cache size                         237.6 MB
max cache size                       5.0 GB

I think at least we can set the max cache size to 150MB or 200MB at most, as a fresh build of v6, v8, v10, v11 generates about 96.3, 110.7, 130.3 & 129.3MB, 150MB should be enough for a while, and the files being evicted should be those we don't need, I'll send a PR for it.

@PeterDaveHello
Copy link
Member Author

PR sent here: #927

@PeterDaveHello
Copy link
Member Author

Even test with v6.10.0 & v6.10.1 which contains smaller changes:

cache directory                     /peter/.ccache
primary config                      /peter/.ccache/ccache.conf
secondary config      (readonly)    /etc/ccache.conf
stats zero time                     Thu Nov 22 06:55:12 2018
cache hit (direct)                     0
cache hit (preprocessed)               0
cache miss                          4174
cache hit rate                      0.00 %
called for link                       16
called for preprocessing               6
ccache internal error                 52
no input file                          2
cleanups performed                     0
files in cache                     12681
cache size                         181.0 MB
max cache size                       5.0 GB

@PeterDaveHello
Copy link
Member Author

Reimplement again in #927

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.

3 participants