Buildbots roundtable notes US LLVM Dev 2022 (original) (raw)

November 23, 2022, 3:50pm 15

Hi, I would like to chime in on this discussion and add my thoughts that I’ve already shared with my team (@nikic , @tstellar , @serge-sans-paille , @tbaeder , @cuviper ) and with the LLVM IWG (@mehdi_amini ) and @gkistanova .

First I think we must salute to the buildbot fleet maintainers and @gkistanova for providing such a nice variety of build and test flavours. This is great and I think we have a pretty decent post-merge setup right now and lots of issues can be found with it!

I heard from my colleagues that sometimes builders fail that you don’t know about or expected them to fail. They mentioned that it would be nice if you could re-run tests on those builder on demand.

I don’t want to worry so much about the time a pre-merge test takes to complete as long as it is not enforced.

People before me have mentioned that the current pre-commit tests in Phabricator are too flaky. They can find good errors but have their problems and they are completely distinct to the vast fleet of buildbot configurations out there. They represent a distilled set of test configurations, sort of like a lowest common denominator. There’s nothing wrong with it per se.

Nomatter what technology (Phabricator or Github PRs) we use, I think we can borrow from the post-merge buildbot fleet that we already have.

There’s a concept in builtbot called trybot. Given that you have the username and password this concept essentially lets you run a build on a builder of your choice. This sounds scary at first, not very secure and vulnerable to DoS attacks.

But what if you added a layer in between the developer and the trybot feature?

• This would not only make it more secure and protect our buildbot infra for the post-merge tests.
• It would also allow us to federate access to buildbots on a whole different level.
  • Not only could developers have a say “where” to test their patch.
  • We could also decide which buildbots want to participate in the trybot scenario at all (on an opt-in basis for the buildbot maintainers).
  • We could have buildbot owners setup a list of paths that, if touched in a patch, their buildbot is triggered.
• We could collect statistics about which buildbots are often requested or fail the most.
  • Based on these stats we could invest in specific upgrades rather than coming up with a test config that doesn’t meet the real needs and is only worth the paper is is written on.
• The testing infra gets more transparent to developers.
  • The buildbots not only bug you after you’ve merged your patch but they’ll become a team-player to help you get the best.
• If you decide to run tests for your patch on a slow but important buildbot from your perspective, you can. Nobody would prohibit this. Afterall it is your choice.
• Running tests with the same buildbots that we have for post-merge, does make the testing infra less hard to maintain (no buildkite and alike is needed, but it is not excluded).
  • No contractor needs to maintain it but sure can, while the bulk of work is done by buildbots.
• We maintain the independence of companies/communities to provide buildbots that are important to them and even if they are slow they can still use them in pre-merge tests.
• We could move to this approach on a very granular level and improve it over time.
  • In gitlab you’ve so called “quick actions” that are text-based controls for issues and merge requests. I’ve a something similar in mind (and in my drawer) that works with Github PRs.
    * You could start with commands like /test-on-builder XY, /re-run-failed-builder, /test-on-recommended-builder.
    * Depending on what you need this can be extended over time incrementally.
    * You can limit whose allowed to run those commands and for whom they have no effect.
    * Github play the role of federating the access to the trybot feature.
    * FYI, the PoC in my drawer even allows you develop the /commands using a locally isolated buildbot setup that somewhat mimics what we have in LLVM (a buildbot master and several builders). It works by running several containers one of which is a github runner that ties up any upstream github repo with your locally running buildbot setup. I found this was needed because we certainly need to tweak the buildbot setup in some ways.
• The post-merge builtbot fleet can stay intact
  • Buildbot maintainers can toggle a switch to open their buildbot bot a selected list of users/groups for pre-commit testing. No additional effort is need from their end.
• I think there’s no debate that cleaner patches to the main branch help to improve the quality. Once we start to see people begin to value this, the cultural change comes naturally but it is still not enforced.

Given that all of this is opt-in, from a developer’s and buildbot owner’s perspective, IMHO this setup provides what @mehdi_amini called “appealing to users”. I find it non-invasive compared to having a non-debatable pre-merge testing setup, that only runs the lowest common denominator.

I’d love to hear how this sounds to all the skeptics of pre-merge testing and to other critiques.