Hi all, Thanks for the good discussion and all the comments. Overall, it seems that we all agree on the bad state of our CI. That's a good first step!
I have talked to a few folks this week about it and it seems that many folks (including me) are not comfortable with merging PRs at the moment because the results of our builds are so bad. I had 40+ failed tests in one of my PRs, all unrelated to my changes. It is really hard to be productive with this. Personally, I really want to move towards requiring a green build to merge to trunk because this is a clear and binary signal. I agree that we need to stabilize the builds before we could even require this so here is my proposal. 1) We could leverage the `reports.junitXml.mergeReruns` option in gradle. >From the doc [1]: > When mergeReruns is enabled, if a test fails but is then retried and succeeds, its failures will be recorded as <flakyFailure> instead of <failure>, within one <testcase>. This is effectively the reporting produced by the surefire plugin of Apache Maven™ when enabling reruns. If your CI server understands this format, it will indicate that the test was flaky. If it > does not, it will indicate that the test succeeded as it will ignore the <flakyFailure> information. If the test does not succeed (i.e. it fails for every retry), it will be indicated as having failed whether your tool understands this format or not. > When mergeReruns is disabled (the default), each execution of a test will be listed as a separate test case. It would not resolve all the faky tests for sure but it would at least reduce the noise. I see this as a means to get to green builds faster. I played a bit with this setting and I discovered [2]. I was hoping that [3] could help to resolve it but I need to confirm. 2) I suppose that we would still have flaky tests preventing us from getting a green build even with the setting in place. For those, I think that we need to review them one by one and decide whether we want to fix or disable them. This is a short term effort to help us get to green builds. 3) When we get to a point where we can get green builds consistently, we could enforce it. 4) Flaky tests won't disappear with this. They are just hidden. Therefore, we also need a process to review the flaky tests and address them. Here, I think that we could leverage the dashboard shared by Ismael. One possibility would be to review it regularly and decide for each test whether it should be fixed, disabled or even removed. Please let me know what you think. Another angle that we could consider is improving the CI infrastructure as well. I think that many of those flaky tests are due to overloaded Jenkins workers. We should perhaps discuss with the infra team to see whether we could do something there. Best, David [1] https://docs.gradle.org/current/userguide/java_testing.html#mergereruns [2] https://github.com/gradle/gradle/issues/23324 [3] https://github.com/apache/kafka/pull/14687 On Wed, Nov 22, 2023 at 4:10 AM Ismael Juma <m...@ismaeljuma.com> wrote: > Hi, > > We have a dashboard already: > > [image: image.png] > > > https://ge.apache.org/scans/tests?search.names=Git%20branch&search.relativeStartTime=P28D&search.rootProjectNames=kafka&search.timeZoneId=America%2FLos_Angeles&search.values=trunk&tests.sortField=FLAKY > > On Tue, Nov 14, 2023 at 10:41 PM Николай Ижиков <nizhi...@apache.org> > wrote: > >> Hello guys. >> >> I want to tell you about one more approach to deal with flaky tests. >> We adopt this approach in Apache Ignite community, so may be it can be >> helpful for Kafka, also. >> >> TL;DR: Apache Ignite community have a tool that provide a statistic of >> tests and can tell if PR introduces new failures. >> >> Apache Ignite has a many tests. >> Latest «Run All» contains around 75k. >> Most of test has integration style therefore count of flacky are >> significant. >> >> We build a tool - Team City Bot [1] >> That provides a combined statistic of flaky tests [2] >> >> This tool can compare results of Run All for PR and master. >> If all OK one can comment jira ticket with a visa from bot [3] >> >> Visa is a quality proof of PR for Ignite committers. >> And we can sort out most flaky tests and prioritize fixes with the bot >> statistic [2] >> >> TC bot integrated with the Team City only, for now. >> But, if Kafka community interested we can try to integrate it with >> Jenkins. >> >> [1] https://github.com/apache/ignite-teamcity-bot >> [2] https://tcbot2.sbt-ignite-dev.ru/current.html?branch=master&count=10 >> [3] >> https://issues.apache.org/jira/browse/IGNITE-19950?focusedCommentId=17767394&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17767394 >> >> >> >> > 15 нояб. 2023 г., в 09:18, Ismael Juma <m...@ismaeljuma.com> написал(а): >> > >> > To use the pain analogy, people seem to have really good painkillers and >> > hence they somehow don't feel the pain already. ;) >> > >> > The reality is that important and high quality tests will get fixed. >> Poor >> > quality tests (low signal to noise ratio) might not get fixed and >> that's ok. >> > >> > I'm not opposed to marking the tests as release blockers as a starting >> > point, but I'm saying it's fine if people triage them and decide they >> are >> > not blockers. In fact, that has already happened in the past. >> > >> > Ismael >> > >> > On Tue, Nov 14, 2023 at 10:02 PM Matthias J. Sax <mj...@apache.org> >> wrote: >> > >> >> I agree on the test gap argument. However, my worry is, if we don't >> >> "force the pain", it won't get fixed at all. -- I also know, that we >> try >> >> to find an working approach for many years... >> >> >> >> My take is that if we disable a test and file a non-blocking Jira, it's >> >> basically the same as just deleting the test all together and never >> talk >> >> about it again. -- I believe, this is not want we aim for, but we aim >> >> for good test coverage and a way to get these test fixed? >> >> >> >> Thus IMHO we need some forcing function (either keep the tests and feel >> >> the pain on every PR), or disable the test and file a blocker JIRA so >> >> the pain surfaces on a release forcing us to do something about it. >> >> >> >> If there is no forcing function, it basically means we are willing to >> >> accept test gaps forever. >> >> >> >> >> >> -Matthias >> >> >> >> On 11/14/23 9:09 PM, Ismael Juma wrote: >> >>> Matthias, >> >>> >> >>> Flaky tests are worse than useless. I know engineers find it hard to >> >>> disable them because of the supposed test gap (I find it hard too), >> but >> >> the >> >>> truth is that the test gap is already there! No-one blocks merging >> PRs on >> >>> flaky tests, but they do get used to ignoring build failures. >> >>> >> >>> The current approach has been attempted for nearly a decade and it has >> >>> never worked. I think we should try something different. >> >>> >> >>> When it comes to marking flaky tests as release blockers, I don't >> think >> >>> this should be done as a general rule. We should instead assess on a >> case >> >>> by case basis, same way we do it for bugs. >> >>> >> >>> Ismael >> >>> >> >>> On Tue, Nov 14, 2023 at 5:02 PM Matthias J. Sax <mj...@apache.org> >> >> wrote: >> >>> >> >>>> Thanks for starting this discussion David! I totally agree to "no"! >> >>>> >> >>>> I think there is no excuse whatsoever for merging PRs with >> compilation >> >>>> errors (except an honest mistake for conflicting PRs that got merged >> >>>> interleaved). -- Every committer must(!) check the Jenkins status >> before >> >>>> merging to avoid such an issue. >> >>>> >> >>>> Similar for actual permanently broken tests. If there is no green >> build, >> >>>> and the same test failed across multiple Jenkins runs, a committer >> >>>> should detect this and cannot merge a PR. >> >>>> >> >>>> Given the current state of the CI pipeline, it seems possible to get >> >>>> green runs, and thus I support the policy (that we actually always >> had) >> >>>> to only merge if there is at least one green build. If committers got >> >>>> sloppy about this, we need to call it out and put a hold on this >> >> practice. >> >>>> >> >>>> (The only exception from the above policy would be a very unstable >> >>>> status for which getting a green build is not possible at all, due to >> >>>> too many flaky tests -- for this case, I would accept to merge even >> >>>> there is no green build, but committer need to manually ensure that >> >>>> every test did pass in at least one test run. -- We had this in the >> >>>> past, but we I don't think we are in such a bad situation right now). >> >>>> >> >>>> About disabling tests: I was never a fan of this, because in my >> >>>> experience those tests are not fixed any time soon. Especially, >> because >> >>>> we do not consider such tickets as release blockers. To me, seeing >> tests >> >>>> fails on PR build is actually a good forcing function for people to >> feel >> >>>> the pain, and thus get motivated to make time to fix the tests. >> >>>> >> >>>> I have to admit that I was a little bit sloppy paying attention to >> flaky >> >>>> tests recently, and I highly appreciate this effort. Also thanks to >> >>>> everyone how actually filed a ticket! IMHO, we should file a ticket >> for >> >>>> every flaky test, and also keep adding comments each time we see a >> test >> >>>> fail to be able to track the frequency at which a tests fails, so we >> can >> >>>> fix the most pressing ones first. >> >>>> >> >>>> Te me, the best forcing function to get test stabilized is to file >> >>>> tickets and consider them release blockers. Disabling tests does not >> >>>> really help much IMHO to tackle the problem (we can of course still >> >>>> disable them to get noise out of the system, but it would only >> introduce >> >>>> testing gaps for the time being and also does not help to figure out >> how >> >>>> often a test fails, so it's not a solution to the problem IMHO) >> >>>> >> >>>> >> >>>> -Matthias >> >>>> >> >>>> On 11/13/23 11:40 PM, Sagar wrote: >> >>>>> Hi Divij, >> >>>>> >> >>>>> I think this proposal overall makes sense. My only nit sort of a >> >>>> suggestion >> >>>>> is that let's also consider a label called newbie++[1] for flaky >> tests >> >> if >> >>>>> we are considering adding newbie as a label. I think some of the >> flaky >> >>>>> tests need familiarity with the codebase or the test setup so as a >> >> first >> >>>>> time contributor, it might be difficult. newbie++ IMO covers that >> >> aspect. >> >>>>> >> >>>>> [1] >> >>>>> >> >>>> >> >> >> https://issues.apache.org/jira/browse/KAFKA-15406?jql=project%20%3D%20KAFKA%20AND%20labels%20%3D%20%22newbie%2B%2B%22 >> >>>>> >> >>>>> Let me know what you think. >> >>>>> >> >>>>> Thanks! >> >>>>> Sagar. >> >>>>> >> >>>>> On Mon, Nov 13, 2023 at 9:11 PM Divij Vaidya < >> divijvaidy...@gmail.com> >> >>>>> wrote: >> >>>>> >> >>>>>>> Please, do it. >> >>>>>> We can use specific labels to effectively filter those tickets. >> >>>>>> >> >>>>>> We already have a label and a way to discover flaky tests. They are >> >>>> tagged >> >>>>>> with the label "flaky-test" [1]. There is also a label "newbie" [2] >> >>>> meant >> >>>>>> for folks who are new to Apache Kafka code base. >> >>>>>> My suggestion is to send a broader email to the community (since >> many >> >>>> will >> >>>>>> miss details in this thread) and call for action for committers to >> >>>>>> volunteer as "shepherds" for these tickets. I can send one out >> once we >> >>>> have >> >>>>>> some consensus wrt next steps in this thread. >> >>>>>> >> >>>>>> >> >>>>>> [1] >> >>>>>> >> >>>>>> >> >>>> >> >> >> https://issues.apache.org/jira/browse/KAFKA-13421?jql=project%20%3D%20KAFKA%20AND%20status%20in%20(Open%2C%20%22In%20Progress%22%2C%20Reopened%2C%20%22Patch%20Available%22)%20AND%20resolution%20%3D%20Unresolved%20AND%20labels%20%3D%20flaky-test%20ORDER%20BY%20priority%20DESC%2C%20updated%20DESC >> >>>>>> >> >>>>>> >> >>>>>> [2] https://kafka.apache.org/contributing -> Finding a project to >> >> work >> >>>> on >> >>>>>> >> >>>>>> >> >>>>>> Divij Vaidya >> >>>>>> >> >>>>>> >> >>>>>> >> >>>>>> On Mon, Nov 13, 2023 at 4:24 PM Николай Ижиков < >> nizhi...@apache.org> >> >>>>>> wrote: >> >>>>>> >> >>>>>>> >> >>>>>>>> To kickstart this effort, we can publish a list of such tickets >> in >> >> the >> >>>>>>> community and assign one or more committers the role of a >> «shepherd" >> >>>> for >> >>>>>>> each ticket. >> >>>>>>> >> >>>>>>> Please, do it. >> >>>>>>> We can use specific label to effectively filter those tickets. >> >>>>>>> >> >>>>>>>> 13 нояб. 2023 г., в 15:16, Divij Vaidya <divijvaidy...@gmail.com >> > >> >>>>>>> написал(а): >> >>>>>>>> >> >>>>>>>> Thanks for bringing this up David. >> >>>>>>>> >> >>>>>>>> My primary concern revolves around the possibility that the >> >> currently >> >>>>>>>> disabled tests may remain inactive indefinitely. We currently >> have >> >>>>>>>> unresolved JIRA tickets for flaky tests that have been pending >> for >> >> an >> >>>>>>>> extended period. I am inclined to support the idea of disabling >> >> these >> >>>>>>> tests >> >>>>>>>> temporarily and merging changes only when the build is >> successful, >> >>>>>>> provided >> >>>>>>>> there is a clear plan for re-enabling them in the future. >> >>>>>>>> >> >>>>>>>> To address this issue, I propose the following measures: >> >>>>>>>> >> >>>>>>>> 1\ Foster a supportive environment for new contributors within >> the >> >>>>>>>> community, encouraging them to take on tickets associated with >> flaky >> >>>>>>> tests. >> >>>>>>>> This initiative would require individuals familiar with the >> relevant >> >>>>>> code >> >>>>>>>> to offer guidance to those undertaking these tasks. Committers >> >> should >> >>>>>>>> prioritize reviewing and addressing these tickets within their >> >>>>>> available >> >>>>>>>> bandwidth. To kickstart this effort, we can publish a list of >> such >> >>>>>>> tickets >> >>>>>>>> in the community and assign one or more committers the role of a >> >>>>>>> "shepherd" >> >>>>>>>> for each ticket. >> >>>>>>>> >> >>>>>>>> 2\ Implement a policy to block minor version releases until the >> >>>> Release >> >>>>>>>> Manager (RM) is satisfied that the disabled tests do not result >> in >> >>>> gaps >> >>>>>>> in >> >>>>>>>> our testing coverage. The RM may rely on Subject Matter Experts >> >> (SMEs) >> >>>>>> in >> >>>>>>>> the specific code areas to provide assurance before giving the >> green >> >>>>>>> light >> >>>>>>>> for a release. >> >>>>>>>> >> >>>>>>>> 3\ Set a community-wide goal for 2024 to achieve a stable >> Continuous >> >>>>>>>> Integration (CI) system. This goal should encompass projects >> such as >> >>>>>>>> refining our test suite to eliminate flakiness and addressing >> >>>>>>>> infrastructure issues if necessary. By publishing this goal, we >> >> create >> >>>>>> a >> >>>>>>>> shared vision for the community in 2024, fostering alignment on >> our >> >>>>>>>> objectives. This alignment will aid in prioritizing tasks for >> >>>> community >> >>>>>>>> members and guide reviewers in allocating their bandwidth >> >> effectively. >> >>>>>>>> >> >>>>>>>> -- >> >>>>>>>> Divij Vaidya >> >>>>>>>> >> >>>>>>>> >> >>>>>>>> >> >>>>>>>> On Sun, Nov 12, 2023 at 2:58 AM Justine Olshan >> >>>>>>> <jols...@confluent.io.invalid> >> >>>>>>>> wrote: >> >>>>>>>> >> >>>>>>>>> I will say that I have also seen tests that seem to be more >> flaky >> >>>>>>>>> intermittently. It may be ok for some time and suddenly the CI >> is >> >>>>>>>>> overloaded and we see issues. >> >>>>>>>>> I have also seen the CI struggling with running out of space >> >>>> recently, >> >>>>>>> so I >> >>>>>>>>> wonder if we can also try to improve things on that front. >> >>>>>>>>> >> >>>>>>>>> FWIW, I noticed, filed, or commented on several flaky test JIRAs >> >> last >> >>>>>>> week. >> >>>>>>>>> I'm happy to try to get to green builds, but everyone needs to >> be >> >> on >> >>>>>>> board. >> >>>>>>>>> >> >>>>>>>>> https://issues.apache.org/jira/browse/KAFKA-15529 >> >>>>>>>>> https://issues.apache.org/jira/browse/KAFKA-14806 >> >>>>>>>>> https://issues.apache.org/jira/browse/KAFKA-14249 >> >>>>>>>>> https://issues.apache.org/jira/browse/KAFKA-15798 >> >>>>>>>>> https://issues.apache.org/jira/browse/KAFKA-15797 >> >>>>>>>>> https://issues.apache.org/jira/browse/KAFKA-15690 >> >>>>>>>>> https://issues.apache.org/jira/browse/KAFKA-15699 >> >>>>>>>>> https://issues.apache.org/jira/browse/KAFKA-15772 >> >>>>>>>>> https://issues.apache.org/jira/browse/KAFKA-15759 >> >>>>>>>>> https://issues.apache.org/jira/browse/KAFKA-15760 >> >>>>>>>>> https://issues.apache.org/jira/browse/KAFKA-15700 >> >>>>>>>>> >> >>>>>>>>> I've also seen that kraft transactions tests often flakily see >> that >> >>>>>> the >> >>>>>>>>> producer id is not allocated and times out. >> >>>>>>>>> I can file a JIRA for that too. >> >>>>>>>>> >> >>>>>>>>> Hopefully this is a place we can start from. >> >>>>>>>>> >> >>>>>>>>> Justine >> >>>>>>>>> >> >>>>>>>>> On Sat, Nov 11, 2023 at 11:35 AM Ismael Juma <m...@ismaeljuma.com >> > >> >>>>>> wrote: >> >>>>>>>>> >> >>>>>>>>>> On Sat, Nov 11, 2023 at 10:32 AM John Roesler < >> >> vvcep...@apache.org> >> >>>>>>>>> wrote: >> >>>>>>>>>> >> >>>>>>>>>>> In other words, I’m biased to think that new flakiness >> indicates >> >>>>>>>>>>> non-deterministic bugs more often than it indicates a bad >> test. >> >>>>>>>>>>> >> >>>>>>>>>> >> >>>>>>>>>> My experience is exactly the opposite. As someone who has >> tracked >> >>>>>> many >> >>>>>>> of >> >>>>>>>>>> the flaky fixes, the vast majority of the time they are an >> issue >> >>>> with >> >>>>>>> the >> >>>>>>>>>> test. >> >>>>>>>>>> >> >>>>>>>>>> Ismael >> >>>>>>>>>> >> >>>>>>>>> >> >>>>>>> >> >>>>>>> >> >>>>>> >> >>>>> >> >>>> >> >>> >> >> >> >>
