This change will help with test time since it will increase cache hits for the Gradle test tasks. For example, if your PR only changes streams code you should ideally not see other modules get re-tested (unless they are downstream dependencies of :streams).
I've learned from ASF Infra that merge queues will be available soon. In light of this, I'm going to expand this proposal into a KIP that includes merge queues. Stay tuned -David A On Wed, Jan 22, 2025 at 6:37 PM Sophie Blee-Goldman <sop...@responsive.dev> wrote: > David, just to make sure I understand, are you saying this will help with > the actual test runtime, or only the compile time of the build? Just > wondering because I've personally never found the compile times to be a > roadblock, since the test times are so long. If this change only speeds up > compiling then it's hardly going to be noticeable imho > > -Sophie > > On Fri, Jan 17, 2025 at 5:16 PM David Arthur <mum...@gmail.com> wrote: > > > Thanks for raising the correctness aspect, José. We currently have a > > compromise solution since PRs are not required to be up-to-date with > trunk > > before merging. Basically, as long as a PR does not have conflicts and > has > > passing tests, it can be merged. That's the situation today. We rely on > the > > trunk builds to retroactively inform us about problems. > > > > The merge queue is probably the solution to your concern. This would > allow > > us to guarantee that each PR has run the tests with the latest code on > > trunk one at a time. We still can't turn this feature on, but I'm hopeful > > it will be available to us soon. > > > > For this PR, I have kept the "Compile and Check Java" step building the > > merge ref (trunk + PR). This will retain some of the previous behavior > and > > at least help prevent breaking trunk due to compilation errors. > > > > In order to benefit from the build cache, contributors will need to pull > in > > trunk often. I'm hoping this is a good incentive to keep the branch > > updated. > > > > > I never found that the limiting factor in merging a PR to Apache Kafka > is > > the build times. > > > > I have :) > > > > It really depends on the nature of the change. Right now, the community > is > > driving a lot of small cleanup/removal PRs for 4.0 which are easy to > review > > but take time to test and build (and re-build). > > > > -David A > > > > On Fri, Jan 17, 2025 at 4:30 PM José Armando García Sancio > > <jsan...@confluent.io.invalid> wrote: > > > > > Thanks David. > > > > > > On Fri, Jan 17, 2025 at 11:55 AM David Arthur <mum...@gmail.com> > wrote: > > > > > > > > I have a simple patch which changes our PRs to build the HEAD of the > PR > > > > rather than the merge ref. > > > > > > > > https://github.com/apache/kafka/pull/18449 > > > > > > We shouldn't sacrifice correctness for performance. The tests need to > > > run against what has been committed to the trunk branch. Otherwise the > > > onus is on the contributor and committer to make sure that the PR > > > branch is up to date enough. > > > > > > I never found that the limiting factor in merging a PR to Apache Kafka > > > is the build times. The limiting factor is committer review time. > > > > > > Thanks, > > > -- > > > -José > > > > > > > > > -- > > David Arthur > > > -- David Arthur
