-
Notifications
You must be signed in to change notification settings - Fork 14k
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
KAFKA-12728: version upgrades: gradle (6.8.3 -->> 7.0.2) and gradle shadow plugin (6.1.0 -->> 7.0.0) #10606
Conversation
@ijuma Gradle 7.0.1 will be released in a few days: https://github.com/gradle/gradle/milestone/173 and after that I will open this PR for a review. Note: Gradle-7-compatible shadow plugin version is also released: https://github.com/johnrengelman/shadow/releases/tag/7.0.0 but I would pass on that one (at least for this PR). |
Any reason not to include the shadow upgrade? If it's reasonably low risk, we can include it here. |
I would argue that risk is low/acceptable: potential issues with shadow plugin new version should not pass unnoticed that easily (it is a shadow plugin after all; in case that shadowing goes south some tests will surely fail). So, we have a deal then: for a next few days I will look for some shadow-plugin blocker issues and in case I don't find any I will include this upgrade as well. |
Sounds good. |
Looks like there is a change in behavior in Gradle 7 related to resource files that's causing a bunch of tests to fail |
Indeed... on my local machine I received around 1% of broken tests (and if I recall correctly they were all related to SSL/TLS, so I was hoping for a better test results here). Back to drawing board, I guess (I will execute tests again on my side with or without gradle shadow upgrade and then compare with test results here). Changing PR status to draft (again). |
I don't think the issues are related to the shadow plugin. They're related to the gradle upgrade. |
Probably they are... I will try to isolate issue(s) and compare it with Gradle 7 release notes. |
Note: I am trying to exact JDK version for Github generated Jenkins build: https://ci-builds.apache.org/blue/organizations/jenkins/Kafka%2Fkafka-pr/detail/PR-10606/3/pipeline/16 becouse it seems that I have more recent Java 8 version installed. Test that fail here on Jenkins: https://ci-builds.apache.org/blue/organizations/jenkins/Kafka%2Fkafka-pr/detail/PR-10606/1/tests Around 50 test fail only on my local box: Gradle command: Environment:
All failed test stacktrace starts like this:
It seems that these other people have similar issues:
Also, Oracle opened few JIRA related tickets:
|
Also... both here on Jenkins and on my laptop there is another type of problems with some test failing like this:
There are some similar problems recorded (although with Maven... but luckily I'm a Maven aficionado, too 😃):
I will investigate this also (could be some regression in Gradle 7). |
Short update:
|
Ok, closing the gap here:
|
@dejan2609 did the exclusion stop working with the upgrade to Gradle 7.0? That seems like the most plausible explanation I've seen so far. |
Thing is that exclusion covers I think I managed to get around this (just finished testing on my end). |
So, just to sum it up:
|
PR rebased, commits are squashed into one; tests on Jenkins are looking good: it seems that only one unrelated test failed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. The JDK 15 job failed due to a timeout, I restarted the build.
FYI/note: Gradle 7.0.2 hotfix is released few hours ago (it solves one corner case on Alpine Linux): https://github.com/gradle/gradle/releases/tag/v7.0.2 Let me know if you want me to bump Gradle to 7.0.2 (but it also makes sense to skip this, because issue affects only one Linux distro). |
Let's bump it. |
…hadow plugin (6.1.0 -->> 7.0.0) details: * gradle upgrade: 6.8.3 -->> 7.0.2 ** https://github.com/gradle/gradle/releases/tag/v7.0.0 ** https://github.com/gradle/gradle/releases/tag/v7.0.1 ** https://github.com/gradle/gradle/releases/tag/v7.0.2 * 'distributionSha256Sum' gradle property is included into 'gradle-wrapper.properties' file * gradle shadow plugin upgrade: 6.1.0 -->> 7.0.0 ** https://github.com/johnrengelman/shadow/releases/tag/7.0.0 * remaining configurations obsoleted in Gradle 6 (and removed in Gradle 7) are replaced: ** `compile` -->> `implementation` ** `testCompile` -->> `testImplementation`
Bumped/force-pushed (I took the liberty to skipped local testing on my end just to save some time). |
Test result on Jenkins shows no failures. |
Thanks for the PR! |
…e-allocations-lz4 * apache-github/trunk: (155 commits) KAFKA-12728: Upgrade gradle to 7.0.2 and shadow to 7.0.0 (apache#10606) KAFKA-12778: Fix QuorumController request timeouts and electLeaders (apache#10688) KAFKA-12754: Improve endOffsets for TaskMetadata (apache#10634) Rework on KAFKA-3968: fsync the parent directory of a segment file when the file is created (apache#10680) MINOR: set replication.factor to 1 to make StreamsBrokerCompatibilityService work with old broker (apache#10673) MINOR: prevent cleanup() from being called while Streams is still shutting down (apache#10666) KAFKA-8326: Introduce List Serde (apache#6592) KAFKA-12697: Add Global Topic and Partition count metrics to the Quorum Controller (apache#10679) KAFKA-12648: MINOR - Add TopologyMetadata.Subtopology class for subtopology metadata (apache#10676) MINOR: Update jacoco to 0.8.7 for JDK 16 support (apache#10654) MINOR: exclude all `src/generated` and `src/generated-test` (apache#10671) KAFKA-12772: Move all transaction state transition rules into their states (apache#10667) KAFKA-12758 Added `server-common` module to have server side common classes. (apache#10638) MINOR Removed copying storage libraries specifically as they are already copied. (apache#10647) KAFKA-5876: KIP-216 Part 4, Apply InvalidStateStorePartitionException for Interactive Queries (apache#10657) KAFKA-12747: Fix flakiness in shouldReturnUUIDsWithStringPrefix (apache#10643) MINOR: remove unnecessary placeholder from WorkerSourceTask#recordSent (apache#10659) MINOR: Remove unused `scalatest` definition from `dependencies.gradle` (apache#10655) MINOR: checkstyle version upgrade: 8.20 -> 8.36.2 (apache#10656) KAFKA-12464: minor code cleanup and additional logging in constrained sticky assignment (apache#10645) ...
JIRA ticket: https://issues.apache.org/jira/browse/KAFKA-12728
Related pull requests:
copyDependentLibs
should not copy testRuntime configuration jars #10466Note: we will wait for a Gradle 7.0.1 patch◀️