You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "gharris1727 (via GitHub)" <gi...@apache.org> on 2024/05/23 18:16:12 UTC
[PR] MINOR: Disable JDK 11 and 17 builds on PRs [kafka]
gharris1727 opened a new pull request, #16051:
URL: https://github.com/apache/kafka/pull/16051
Testing these JDK versions in PRs is low value, and has a high cost in CI workload. At least temporarily we should disable these, and only run the full JDK suites for branch builds.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Disable JDK 11 and 17 tests on PRs [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on PR #16051:
URL: https://github.com/apache/kafka/pull/16051#issuecomment-2128174644
> > Something is wrong with the build of this commit
>
> There still seems to be some issue with builds 🤔
> [2024-05-23T21:45:16.078Z] Could not stop org.gradle.internal.actor.internal.DefaultActorFactory$NonBlockingActor@1d0918ec.
That was probably me. Marking the PR as ready-for-review forcefully killed the previous build, and the next build is still waiting in the queue. I think that the status message should say "queued", and it just wasn't prepared for this strange state. If it doesn't resolve in a few hours then we might want to be worried :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Disable JDK 11 and 17 builds on PRs [kafka]
Posted by "jolshan (via GitHub)" <gi...@apache.org>.
jolshan commented on PR #16051:
URL: https://github.com/apache/kafka/pull/16051#issuecomment-2127798238
I was just having a conversation about this with some folks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Disable JDK 11 and 17 tests on PRs [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #16051:
URL: https://github.com/apache/kafka/pull/16051#issuecomment-2132080935
I'm ok going with something like this to start with.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Disable JDK 11 and 17 builds on PRs [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #16051:
URL: https://github.com/apache/kafka/pull/16051#issuecomment-2127820090
I think it would be worth running the compile steps for these older versions (i.e., "doValidation"). As far as I know, those steps only fail if there's actually a problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Disable JDK 11 and 17 tests on PRs [kafka]
Posted by "jolshan (via GitHub)" <gi...@apache.org>.
jolshan commented on PR #16051:
URL: https://github.com/apache/kafka/pull/16051#issuecomment-2128160293
Any ideas about
```
[2024-05-23T21:45:16.078Z] > Task :connect:runtime:test
[2024-05-23T21:45:16.078Z] Could not stop org.gradle.internal.actor.internal.DefaultActorFactory$NonBlockingActor@1d0918ec.
[2024-05-23T21:45:16.078Z] org.gradle.internal.dispatch.DispatchException: Could not dispatch message [MethodInvocation method: stop()].
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Disable JDK 11 and 17 tests on PRs [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on PR #16051:
URL: https://github.com/apache/kafka/pull/16051#issuecomment-2129998314
Hey @jolshan @mumrah @ijuma PTAL at the build to see the effect, thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Disable JDK 11 and 17 tests on PRs [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on PR #16051:
URL: https://github.com/apache/kafka/pull/16051#issuecomment-2128068313
Looking at the latest CI run, the 11 build took ~15 minutes, the 17 build took 11 minutes, and the 8 and 21 builds are continuing as normal.
In the last 28 days, Kafka has run:
9380 builds in total
8514 builds on PRs
748 builds on trunk
111 builds on 3.7
7 builds on 3.6
-> 90% of the builds are on PRs
I think we can expect more than a 40% reduction in our total CI load after this change, and I don't think we're losing any test coverage. I'm comfortable with this change as-is, and we can pursue further reductions as they become desired.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Disable JDK 11 and 17 tests on PRs [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on code in PR #16051:
URL: https://github.com/apache/kafka/pull/16051#discussion_r1612327285
##########
Jenkinsfile:
##########
@@ -36,6 +36,12 @@ def doTest(env, target = "test") {
junit '**/build/test-results/**/TEST-*.xml'
}
+def runTestOnDevBranch(env) {
+ if (!isChangeRequest(env)) {
+ doTest(env)
+ }
+}
Review Comment:
What do you think about leaving the 11 and 17 branch builders as-is until 4.0? This will need some rework then to effect the KIPs that are changing the minimum version requirements and having different minimums for the different modules. We can figure out a strategy for branch & pr builds then.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Disable JDK 11 and 17 tests on PRs [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #16051:
URL: https://github.com/apache/kafka/pull/16051#discussion_r1612341398
##########
Jenkinsfile:
##########
@@ -36,6 +36,12 @@ def doTest(env, target = "test") {
junit '**/build/test-results/**/TEST-*.xml'
}
+def runTestOnDevBranch(env) {
+ if (!isChangeRequest(env)) {
+ doTest(env)
+ }
+}
Review Comment:
I don't see the value - once we drop support for Java 8, we will simply change the Java 8 builder to Java 11. Am I missing something?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Disable JDK 11 and 17 tests on PRs [kafka]
Posted by "jolshan (via GitHub)" <gi...@apache.org>.
jolshan commented on PR #16051:
URL: https://github.com/apache/kafka/pull/16051#issuecomment-2128158835
> Something is wrong with the build of this commit
There still seems to be some issue with builds 🤔
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Disable JDK 11 and 17 tests on PRs [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on PR #16051:
URL: https://github.com/apache/kafka/pull/16051#issuecomment-2132294825
> Can we increase the parallel number from 2 to 4
I think that is a separate concern. Before or after this change, each "stage" in the Jenkins build gets either a full or half node, and so has the same resource limitations. What this changes is how long we reserve the half-node, and how soon another stage (of another Kafka build, or another project's build) can start running.
I still need to run some more experiments in #16009 to see if we need to change maxParallelForks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Disable JDK 11 and 17 tests on PRs [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on PR #16051:
URL: https://github.com/apache/kafka/pull/16051#issuecomment-2128037835
> It's rare that there is an issue with only java 11/17 and not 8 or 21. I've most frequently seen issues with scala 2.12 and that's on the java 8 build.
I agree, and that was why I picked 11 and 17 to affect here.
> If we still do the compile step, we still need the nodes right?
We would still need the nodes, but instead of 4 nodes for 3 hours each (12 node-hours) we would need 2 nodes for 3 hours and 2 nodes for ~15 minutes (6.5 node-hours).
I'm not very familiar with the jenkins pipeline declaration syntax, and I'm not sure how to express this change in a way which doesn't generate 11 & 17 builds for PRs at all, because it seems like it can only evaluate conditions like isChangeRequest after the stage is already defined. Maybe someone that is more familiar with jenkins can suggest an alternative.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Disable JDK 11 and 17 tests on PRs [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on code in PR #16051:
URL: https://github.com/apache/kafka/pull/16051#discussion_r1612358604
##########
Jenkinsfile:
##########
@@ -36,6 +36,12 @@ def doTest(env, target = "test") {
junit '**/build/test-results/**/TEST-*.xml'
}
+def runTestOnDevBranch(env) {
+ if (!isChangeRequest(env)) {
+ doTest(env)
+ }
+}
Review Comment:
I'm hesitant to sign up future release managers for additional verification of the 11 and 17 builds that the CI has been doing for previous releases. I don't expect any failures to be introduced that affect 11 and 17 but not 8 or 21, but I would rather find those failures out in CI after merge than during the RC generation stage.
And for selfish reasons, I also would like to continue seeing the 11 and 17 trunk results published so I can do flaky test analysis. With this patch as-is, we're keeping the same number of test runs on trunk, but if we removed these entirely we would be cutting those in half. I don't pay attention to the PR builds for statistics because contributors may have bad patches applied that aren't worth investigating.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Disable JDK 11 and 17 tests on PRs [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #16051:
URL: https://github.com/apache/kafka/pull/16051#discussion_r1612342902
##########
Jenkinsfile:
##########
@@ -36,6 +36,12 @@ def doTest(env, target = "test") {
junit '**/build/test-results/**/TEST-*.xml'
}
+def runTestOnDevBranch(env) {
+ if (!isChangeRequest(env)) {
+ doTest(env)
+ }
+}
Review Comment:
Ah, yes - we'll also have to configure the brokers, etc. to use Java 17. In any case, I think whatever we have to do there won't be any harder if we just delete the Java 11 and 17 builders for now. And it will make the 3.8 builds lighter.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Disable JDK 11 and 17 tests on PRs [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #16051:
URL: https://github.com/apache/kafka/pull/16051#discussion_r1612302312
##########
Jenkinsfile:
##########
@@ -36,6 +36,12 @@ def doTest(env, target = "test") {
junit '**/build/test-results/**/TEST-*.xml'
}
+def runTestOnDevBranch(env) {
+ if (!isChangeRequest(env)) {
+ doTest(env)
+ }
+}
Review Comment:
If they don't run on PRs, I don't think anyone will validate the results for the branch builders. We should probably remove Java 11 and 17 altogether now that we have ported most tests to run with Java 16 and higher (previously many were excluded).
I had suggested this in an internal conversation, so good to see similar action here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Disable JDK 11 and 17 tests on PRs [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #16051:
URL: https://github.com/apache/kafka/pull/16051#issuecomment-2132191527
@gharris1727 nice patch. One question: Can we increase the parallel number from 2 to 4 after we disable two suits (11 and 17) of tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Disable JDK 11 and 17 tests on PRs [kafka]
Posted by "jolshan (via GitHub)" <gi...@apache.org>.
jolshan commented on PR #16051:
URL: https://github.com/apache/kafka/pull/16051#issuecomment-2128028213
> I think it would be worth running the compile steps for these older versions (i.e., "doValidation"). As far as I know, those steps only fail if there's actually a problem.
It's rare that there is an issue with only java 11/17 and not 8 or 21. I've most frequently seen issues with scala 2.12 and that's on the java 8 build. If we still do the compile step, we still need the nodes right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Disable JDK 11 and 17 tests on PRs [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 merged PR #16051:
URL: https://github.com/apache/kafka/pull/16051
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org