Issue #16003: Add Kafka to no-error testing#18263
Conversation
|
The daemon indeed runs out of memory on CircleCI runners: I executed the same CircleCI job successfully locally and noticed that the memory usage gradually climbs to ~10 GB (sometimes more): |
|
I notice a lot of |
|
Looking at the task dependency graph via ./gradlew checkstyleMain checkstyleTest --task-graphwe can see that Ideally, this task graph should be flat and contain only |
|
I find it strange that the Checkstyle tasks depend on the |
|
It seems like the Gradle Checkstyle plugin creates an implicit dependency on the compiled files via this task.setClasspath(sourceSet.getOutput().plus(sourceSet.getCompileClasspath()));Related issues:
To get rid of the dependency on the compilation tasks, we should use this (as described here) tasks.withType<Checkstyle>().configureEach {
classpath = files()
} |
|
Well, removing the compilation tasks doesn't solve the memory issues: |
|
Side note for later
|
|
I tried playing with the
Kafka increased their I tried setting lower max values such as |
|
Side note: Apart from GitHub Actions, another option is to configure a more powerful CircleCI container. Though, that depends on the pricing plan that Checkstyle has with CircleCI: https://circleci.com/docs/reference/configuration-reference/#docker-execution-environment: 
|
|
The following approach of running all checkstyle tasks sequentially "works" on CircleCI but comes with the tradeoff of increased runtime (14 minutes 56 seconds on CI) and the daemon respawning 2 times due to lack of memory: no-error-kafka)
CS_POM_VERSION="$(getCheckstylePomVersion)"
echo "CS_version: ${CS_POM_VERSION}"
./mvnw -e --no-transfer-progress clean install -Pno-validations
echo "Checkout target sources ..."
checkout_from "https://github.com/apache/kafka.git"
cd .ci-temp/kafka/
cat >> customConfig.gradle<< EOF
allprojects {
repositories {
mavenLocal()
}
gradle.projectsEvaluated {
tasks.withType(Checkstyle) {
classpath = files()
maxHeapSize = "512m"
}
}
}
EOF
mapfile -t tasks < <(
./gradlew checkstyleMain checkstyleTest \
--task-graph \
-PcheckstyleVersion="${CS_POM_VERSION}" \
-I customConfig.gradle \
| grep -E 'checkstyle(Main|Test)' \
| grep -Eo ':(.+:)+(checkstyleMain|checkstyleTest)'
)
for task in "${tasks[@]}"
do
./gradlew "${task}" \
-PcheckstyleVersion="${CS_POM_VERSION}" \
-I customConfig.gradle || true
done
cd ..
removeFolderWithProtectedFiles kafka
;;Here's a graph from a local run: |
yes
checkstyle always should run after compilation. Just to be sure that all target text files are compiled.
no problem, CI just need to be configured to cache folder. |
|
I'm out of ideas here. Disabling parallelization ( I'll go with GitHub Actions. It should (hopefully) work since the Kafka project already does that. The GitHub runners have 16 GiB of RAM which has to be plenty |
do we have leak in Checkstyle ? It might be possible.
we are free plan.
we can increase wait time for special CI job, just to make it work. |
|
@dejan2609 , can you suggest something here ? Levski (0) CSKA (1) |
|
I moved the job to GitHub Actions, and it works 🚀 However, there are some violations: How do we proceed with that? |
🎉 @stoyanK7 Don't worry about these CheckStyle violations: you are probably compiling against most recent ChekStyle version while Kafka is still using CheckStyle 10.20.2 (I have started this version upgrade PR and I'm waiting for a review from the Kafka maintainers. Hint: if you want to be 💯 percent sure just pass CS 10.20.2 version to your GA workflow 😉 |
@romani @stoyanK7 I would suggest a celebration 😃 |
|
@stoyanK7 , please use such branch to make sure all works, we can even merge it, and eventually such PR is merged and we can remove extra checkout line in script. |
|
@dejan2609 @romani PR is ready for review. Please poke at it 😃 Actionlint CI failure is unrelated. |
|
Совсем хорошо/Съвсем добре/Сасвим добро 👌 One non-binding LGTM ✅ |
| @@ -0,0 +1,27 @@ | |||
| name: 'no-error-kafka' | |||
There was a problem hiding this comment.
Let's name workflow as "no-error testing"
Rename file to match our existing workflow on no-exception
To match
There was a problem hiding this comment.
Done.
Side note: We must also move no-error-trino under this no-error testing workflow. I will do that in a followup PR (after this gets merged).
romani
left a comment
There was a problem hiding this comment.
thanks a lot !!!!!!!
We did it !!!
It took a while, new partnership is singed.
Resolves issue
Based on the work of:
Added GitHub Actions Workflow for regression testing with Apache Kafka:
no-error-kafka. The job is identical to the otherno-error-*jobs. For instance:checkstyle/.ci/validation.sh
Lines 236 to 247 in 57df2a7
Usually, such jobs use Semaphore and CircleCI, but this one must use GitHub Actions due to high memory usage.