travis: reduce the number of travis builders #2306
Conversation
|
@tamird let's wait for the travis build and see how it goes. |
|
@yiwu-arbug of course. By the way, it looks like you guys are currently aggressively maintaining two branches: master, and 5.4.fb. This is really unfortunate, because it adds to the already extremely congested Travis CI situation in this repo. Could you turn off Travis CI on the non-master branches? It currently takes over a day to get a Travis build. See https://travis-ci.org/facebook/rocksdb/builds (at the time of writing):
|
|
@tamird 5.4.fb etc. are release branches. We need travis build there. We can probably turn off travis for other branches, but they are also useful in case people want to run ac-hoc travis job. |
|
I understand that they're useful in general, but something needs to be done - the build takes over 4 hours on average and the congestion causes huge delays. Perhaps you could do the cherry-picks to the release branches once a day, instead of eagerly whenever anything merges to master? |
|
We only cherry-pick what's needed into release branches. The thing is travis don't have enough resources and we don't have a better alternative to it.. |
|
Right, I understand that, but if you batched your cherry-picks at the end of the day, you would substantially reduce the congestion during daylight hours when people are trying to get work done. This would prevent merge-on-red situations like the one caused by #2269, which has required at least 3 follow-up PRs now. |
|
@tamird updated the pull request - view changes |
|
@tamird updated the pull request - view changes |
|
@tamird updated the pull request - view changes |
|
@tamird updated the pull request - view changes |
|
@tamird updated the pull request - view changes |
|
@tamird updated the pull request - view changes |
This collapses all the "platform dependent" tests into a single travis builder in an effort to reduce overall CI times. These builds currently take a combined 21-23 minutes, but each one has to compile the library, so combining them should yield some time savings (5-10 minutes). Unfortunately the other builders don't duplicate work, so combining them is unlikely to provide benefit.
The osx build already exercises clang, so there's no need to also test clang on Linux. This should save about 30% of CI time. Testing with GCC 6 is nice, but not needed, and saves a further 30% of CI time. Finally, this commit arranges for ccache to actually be used in CI, which it previously was not on osx.
|
Heads up that this is green, and the build took 1h49m compared to the usual 3h29m:
There's a merge conflict with #2127 that I'm fixing now, but this should be good to go. |
|
@tamird updated the pull request - view changes |
|
@yiwu-arbug can you take a look, please? |
| @@ -4,9 +4,6 @@ language: cpp | |||
| os: | |||
| - linux | |||
| - osx | |||
| compiler: | |||
| - clang | |||
There was a problem hiding this comment.
We want to keep clang build. How does it affect total time per your experiment?
There was a problem hiding this comment.
Note that the osx build already tests clang - what's the benefit of also testing it on linux?
I'd expect this to increase compilation time by ~50% (i.e. eliminate 33% savings) since it will roughly double the number of linux workers.
There was a problem hiding this comment.
platform_dependent is not building all tests, so there can be compile error catch by clang but not gcc. Hope this case can be covered.
There was a problem hiding this comment.
Ah, you mean that TEST_GROUP=1 and TEST_GROUP=2 don't run on OSX? I can enable those.
There was a problem hiding this comment.
Or simply building the tests without running them.
There was a problem hiding this comment.
Ah, is there a make target that does that?
There was a problem hiding this comment.
Not sure. You can do make all though.
There was a problem hiding this comment.
@tamird seems TEST_GROUP 1&2 is taking an hour to run. Mind remove them and add make all for osx?
|
LGTM. @siying any comment? |
|
No I don't have any comment.
…________________________________
From: yiwu-arbug <notifications@github.com>
Sent: Friday, May 26, 2017 2:43:25 PM
To: facebook/rocksdb
Cc: Siying Dong; Mention
Subject: Re: [facebook/rocksdb] travis: reduce the number of travis builders (#2306)
LGTM. @siying<https://github.com/siying> any comment?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#2306 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AFFmJwMnWNkbEqEUBNGLXMBCECzl0QJQks5r90d9gaJpZM4Nctej>.
|
|
@tamird updated the pull request - view changes |
|
@tamird updated the pull request - view changes |
Previously, these tests were compiled with clang and run in a separate linux build. Now that we're only testing default compilers on each OS, we need to build these tests on osx to make sure there aren't any compiler-specific mistakes hiding in them.
|
@tamird updated the pull request - view changes |
|
Green again! |
|
@yiwu-arbug has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
Merging. Thank you for your contribution! |
|
🎉
…On May 30, 2017 13:13, "yiwu-arbug" ***@***.***> wrote:
Merging. Thank you for your contribution!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2306 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABdsPNj8WMQFnneu1YdLGxZZhDpKjWLoks5r_E5PgaJpZM4Nctej>
.
|
This collapses all the "platform dependent" tests into a single travis
builder in an effort to reduce overall CI times. These builds currently
take a combined 21-23 minutes, but each one has to compile the library,
so combining them should yield some time savings (5-10 minutes).
Unfortunately the other builders don't duplicate work, so combining
them is unlikely to provide benefit.