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

Group tests by language on travis-ci #1446

Closed
wants to merge 15 commits into from

Conversation

hamiltont
Copy link
Contributor

.travis.yml can now accept forms such as "Python". It continues to also accept forms such as "Python/uwsgi" in case you just want to verify one specific framework.

This PR is currently being tested at https://travis-ci.org/hamiltont/FrameworkBenchmarks/builds/55956802 - I'm on vacatino and did this super quickly, so there may be somethign I've missed and a reason not to merge this.

Should drastically reduce the impact that TFB has on Travis-CI, and should also speed up the time it takes to run all verifications. Negative effect is that it becomes harder to scan the verification logs and see where stuff went wrong, becuase there are going to be logs for all the tests in a language instead of just logs for a single test. Possible negative effects is that we may run into the travis 50-minute tmie limit on some languages. If this happens, I'll just turn TESTDIR into a full-blown regex.

@msmith-techempower @LadyMozzarella if the travis results from my test of this PR check out, this should probably be merged immediately. Travis-ci reached out to us to request that we reduce our load on their freely provided services, and this should reduce our load drastically. Someone probably needs to rebase my branch on a current master, I'm on vaca and haven't pulled your master in a week or two

@hamiltont
Copy link
Contributor Author

Yup, someone changed .travis.yml after I last pulled master, so you'll have to do the merge manually. As long as they didn't introduce an entirely new language, it's safe to just overwrite their changes with mine

@msmith-techempower
Copy link
Member

So, this seems to leverage the change that @methane (I believe, sorry if I'm getting the author wrong) made a while back to allow running all tests of a given language.

So, the travis "work" will now be checking that all of a language is passing for a given pull request? That is, if I make a change to one php framework, all php frameworks/tests will be run by travis while the other language will issue "skips"?

@LadyMozzarella
Copy link
Contributor

Thanks @hamiltont! I think it's important that we try to keep Travis-CI happy. I've stolen your commits, rebased and am running a a Travis run from my fork: Travis Build (just in case). I thought I remembered there being another pull request that was working on doing this, but with quick scanning I'm not seeing it anymore.

@hamiltont
Copy link
Contributor Author

So, the travis "work" will now be checking that all of a language is
passing for a given pull request? That is, if I make a change to one php
framework, all php frameworks/tests will be run by travis while the other
language will issue "skips"?

Correct. A change to one is a change to all. This could be improved to just
rerun the changed frameworks inside each language group - maybe I'll add
that in later tonight if I have time.

By the way, my travis run shows that multiple languages hit the 50 minute
time limit, so I'll need to redactor this to make testdir a regex and do
stuff like TESTDIR=Java/a.*

@hamiltont
Copy link
Contributor Author

Ok, refactored into a regex and brought this up to master. Initial tests show this bringing complete travis-CI tests down from "More than 24 hours" to ~9 hours. That's still a substantial load, and perhaps @msmith-techempower 's idea to only run the tests that have changes within each grouping will help, but it's quite possible this won't be enough of a reduction fro travis-ci to continue allowing us to participate in their free offering. I've broken up Java and C++ into smaller groupings as those languages were taking more than 50 minutes. Someone needs to break up python and php before this can be merged.

Same warning: I don't have time to test well at the moment, so it needs more testing before being merged.

@hamiltont
Copy link
Contributor Author

So this appears to be working. It's not optimal - if you cannot read regular expressions, then .travis.yml just got very confusing. Even if you can, there is a crap top of special character escaping going on. Based on this, it seems to me that using travis-ci is just a stopgap until there is a proper continuous benchmarking toolset. Even with this PR, we are still consuming a nontrivial amount of travis-ci resources and the difficulty of interpreting the travis results has increased to the point that it's infeasible for novices. To understand these results you need to know that there are groupings of frameworks, you need to be able to find your frameworks grouping, and you need to be able to parse out the log files for your framework specifically. This all boils down to a poor experience, especially for people just interested in submitting a quick change and moving on with their lives.

So unless TFB has soem way to pay travis for our usage, I expect that this solution will need to be replaced by a continous benchmarking running on the peak hardware

As for this PR - it's still critical we merge ASAP to reduce our travis load (>24 hours to 9 hours is no trivial reduction), and as soon as the precise groupings are found in the .travis.yml (to avoid the 50 minute limit per job imposed by travis-ci) we should merge

Pinging @bhauer so he's aware that there's work ongoing to reduce the load on travis-ci, in case they contact Techempower looking for updates

@ludovic-gasc
Copy link
Contributor

Maybe, it should be possible to split this big repository in several repositories, for example, by language.
Benchmarker should lives in a separate Git repository, and use Git submodule to include Benchmarker in each language repository.
You should also change this big repository to include each language repository with Git submodule.
You have several tools to manage several Git repositories, Git submodule is one example. Android uses repo to do that.
Moreover, it should be easier to merge PR and delegate for languages you don't have the skills internally.

BTW, I understand it's a cost (time+money) to maintain PEAK infrastructure, but, at least from my point of view, it should be really better if, time to time, the benchmark suite should be launched. It will be easier to detect issues during round preparation, instead of to discover just before the big day, as we had with PostgreSQL.
About PostgreSQL, I think that some performance should be improved, I need to investigate.

@hamiltont
Copy link
Contributor Author

Maybe, it should be possible to split this big repository in several repositories, for example, by language.

I'm not following - this won't reduce our load on travis-ci significantly as we already terminate unnecessary jobs almost immediately.

it should be really better if, time to time, the benchmark suite should be launched

Absolutely agree - and I think the entire TechEmpower team wants this too. The end goal is a system that automates all the running, so every commit to master is benchmarked automatically

@msmith-techempower
Copy link
Member

@hamiltont @bhauer @LadyMozzarella I think we have scheduled a talk with the Travis-CI team, but it is sounding more and more like we need to host our own CI software in order to lighten the load.

@LadyMozzarella
Copy link
Contributor

@msmith-techempower - Yeah, I think that's the case. The project just keeps getting larger and larger and it appears that Travis won't be able to handle what we need for very much longer. Either way I think we need to merge in what we can to lighten the usage in Travis ASAP.

@msmith-techempower
Copy link
Member

There was, at some point, talk about building our own hardware to use Travis-CI (or similar) internally and provide the results externally.

@hamiltont Is this something that we can do? I forget where we landed on this.

@hamiltont
Copy link
Contributor Author

@hamiltont Is this something that we can do?

Totally. Two options that I see - either setup our own travis-ci using their open source codebase, or build an in-house solution specific to the needs of TFB. I'm not sure which would be better, they are both nontrivial

@msmith-techempower
Copy link
Member

I'm inclined to try and get our own travis-ci environment set up using their open source codebase.

@hamiltont
Copy link
Contributor Author

Ok, if none of the travis tests error out here then we can merge this and it will reduce our travis load

- "TESTDIR=Nim"
- "TESTDIR=Perl"
# PHP: cakephp to hhvm
- "TESTDIR=PHP\\\\/[c-h]"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... Wat? Are you partitioning these? Won't this be crazy-difficult to maintain for future additions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because language partitioning is impossible here - it overruns our 50 minute time limit with Travis. And yes, it will be hell to maintain. This is why I'm saying in the conversation thread that we absolutely must replace our usage of the public travis-ci in some way - this is clearly an untenable .travis.yml, but its the only .travis.yml that won't consume huge resources on their hardware

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is a stopgap solution only - it'll get us through the next few months without causing too much trouble for travis-ci, but it needs to be replaced as soon as possible with something better

@hamiltont
Copy link
Contributor Author

Marking as closed as it appears we don't need to do this. I'll keep the branch around as an emergency backup for the time being

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants