You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by "kaijchen (via GitHub)" <gi...@apache.org> on 2023/02/15 05:05:43 UTC

[GitHub] [incubator-uniffle] kaijchen opened a new pull request, #602: chore: speed up CI workflows

kaijchen opened a new pull request, #602:
URL: https://github.com/apache/incubator-uniffle/pull/602

   ### What changes were proposed in this pull request?
   
   1. Split unit test and integration test
   2. Enable maven parallel build (`-T1C`)
   3. Do not use matrix in deploy/kubernetes workflow
   4. Delay some secondary workflows 
   
   ### Why are the changes needed?
   
   #580
   Make CI faster.
   In case of failure, rerun the failed part only.
   
   ### Does this PR introduce _any_ user-facing change?
   
   No.
   
   ### How was this patch tested?
   
   https://github.com/kaijchen/incubator-uniffle/actions/runs/4180474332
   
   <img width="979" alt="image" src="https://user-images.githubusercontent.com/5821159/218936913-30318234-d0c8-48c7-bc9d-beb9f204f3da.png">
   


-- 
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: issues-unsubscribe@uniffle.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org

[GitHub] [incubator-uniffle] kaijchen commented on pull request #602: [#580] chore: speed up CI workflows

Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on PR #602:
URL: https://github.com/apache/incubator-uniffle/pull/602#issuecomment-1431379851

   Thanks @advancedxy for the review.


-- 
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: issues-unsubscribe@uniffle.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org

[GitHub] [incubator-uniffle] kaijchen commented on pull request #602: [#580] chore: speed up CI workflows

Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on PR #602:
URL: https://github.com/apache/incubator-uniffle/pull/602#issuecomment-1431372220

   > Ah, I didn't see that coming since you don't specify `-DskipIT`..
   
   Because integration tests are executed by the `maven-surefire-plugin` also.
   `maven-surefire-plugin` is designed to run UT, and controlled by `-DskipUT`.
   I tried to run integration tests by `maven-failsafe-plugin` in #598, but it was too slow.


-- 
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: issues-unsubscribe@uniffle.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org

[GitHub] [incubator-uniffle] advancedxy commented on a diff in pull request #602: [#580] chore: speed up CI workflows

Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #602:
URL: https://github.com/apache/incubator-uniffle/pull/602#discussion_r1107101943


##########
.github/workflows/parallel.yml:
##########
@@ -79,7 +79,7 @@ jobs:
           mvn-${{ inputs.java-version }}-package-${{ matrix.profile }}-
           mvn-${{ inputs.java-version }}-package-
     - name: Execute `mvn ${{ inputs.maven-args }} -P${{ matrix.profile }}`
-      run: mvn -B -fae ${{ inputs.maven-args }} -P${{ matrix.profile }} | tee /tmp/maven.log
+      run: mvn -T1C -B -fae ${{ inputs.maven-args }} -P${{ matrix.profile }} | tee /tmp/maven.log

Review Comment:
   yeah, maybe. But it maybe flaky test and it's hard to debug.
   But I'm ok with this



-- 
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: issues-unsubscribe@uniffle.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org

[GitHub] [incubator-uniffle] kaijchen commented on pull request #602: [#580] chore: speed up CI workflows

Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on PR #602:
URL: https://github.com/apache/incubator-uniffle/pull/602#issuecomment-1431347078

   > 1. run integration tests run with unit test disabled?
   
   UT is already excluded, some integration tests are too slow. We should improve the speed of them. And we can also split integration tests to multiple packages, to further break down and improve parallelism.
   
   > 2. some common integration tests are run multiple times. is it possible to skip integration test of common and spark-common when running spark3 integration test?
   
   I'm not sure, maybe some tests need and some tests don't.
   Please check and split them into different packages.
   


-- 
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: issues-unsubscribe@uniffle.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org

[GitHub] [incubator-uniffle] advancedxy commented on pull request #602: [#580] chore: speed up CI workflows

Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #602:
URL: https://github.com/apache/incubator-uniffle/pull/602#issuecomment-1431340532

   Thanks @kaijchen for this awesome work. Now the CI is down to 20min.
   However there might be some other improvement:
   1. run integration tests run with unit test disabled?
   2. some common integration tests are run multiple times. is it possible to skip integration test of common and spark-common when running spark3 integration test?


-- 
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: issues-unsubscribe@uniffle.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org

[GitHub] [incubator-uniffle] kaijchen commented on a diff in pull request #602: [#580] chore: speed up CI workflows

Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on code in PR #602:
URL: https://github.com/apache/incubator-uniffle/pull/602#discussion_r1107088440


##########
.github/workflows/parallel.yml:
##########
@@ -79,7 +79,7 @@ jobs:
           mvn-${{ inputs.java-version }}-package-${{ matrix.profile }}-
           mvn-${{ inputs.java-version }}-package-
     - name: Execute `mvn ${{ inputs.maven-args }} -P${{ matrix.profile }}`
-      run: mvn -B -fae ${{ inputs.maven-args }} -P${{ matrix.profile }} | tee /tmp/maven.log
+      run: mvn -T1C -B -fae ${{ inputs.maven-args }} -P${{ matrix.profile }} | tee /tmp/maven.log

Review Comment:
   To run tests in parallel, you will need to set `forkCount` in `maven-surefire-plugin.
   At least this is not causing any problems right now.



-- 
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: issues-unsubscribe@uniffle.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org

[GitHub] [incubator-uniffle] codecov-commenter commented on pull request #602: chore: speed up CI workflows

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #602:
URL: https://github.com/apache/incubator-uniffle/pull/602#issuecomment-1430765423

   # [Codecov](https://codecov.io/gh/apache/incubator-uniffle/pull/602?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#602](https://codecov.io/gh/apache/incubator-uniffle/pull/602?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6605cda) into [master](https://codecov.io/gh/apache/incubator-uniffle/commit/56145c2731e68a3e0e89683691c1da0a8172a5d8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (56145c2) will **increase** coverage by `1.47%`.
   > The diff coverage is `n/a`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #602      +/-   ##
   ============================================
   + Coverage     61.67%   63.15%   +1.47%     
   - Complexity     1668     1801     +133     
   ============================================
     Files           201      200       -1     
     Lines         11111    10416     -695     
     Branches        922     1044     +122     
   ============================================
   - Hits           6853     6578     -275     
   + Misses         3883     3494     -389     
   + Partials        375      344      -31     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-uniffle/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...tor/pkg/controller/sync/coordinator/coordinator.go](https://codecov.io/gh/apache/incubator-uniffle/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL2NvbnRyb2xsZXIvc3luYy9jb29yZGluYXRvci9jb29yZGluYXRvci5nbw==) | | |
   | [...y/kubernetes/operator/pkg/webhook/inspector/pod.go](https://codecov.io/gh/apache/incubator-uniffle/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL3dlYmhvb2svaW5zcGVjdG9yL3BvZC5nbw==) | | |
   | [...y/kubernetes/operator/pkg/webhook/inspector/rss.go](https://codecov.io/gh/apache/incubator-uniffle/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL3dlYmhvb2svaW5zcGVjdG9yL3Jzcy5nbw==) | | |
   | [deploy/kubernetes/operator/pkg/utils/certs.go](https://codecov.io/gh/apache/incubator-uniffle/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL3V0aWxzL2NlcnRzLmdv) | | |
   | [...pkg/controller/sync/shuffleserver/shuffleserver.go](https://codecov.io/gh/apache/incubator-uniffle/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL2NvbnRyb2xsZXIvc3luYy9zaHVmZmxlc2VydmVyL3NodWZmbGVzZXJ2ZXIuZ28=) | | |
   | [...rnetes/operator/pkg/webhook/inspector/inspector.go](https://codecov.io/gh/apache/incubator-uniffle/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL3dlYmhvb2svaW5zcGVjdG9yL2luc3BlY3Rvci5nbw==) | | |
   | [deploy/kubernetes/operator/pkg/utils/util.go](https://codecov.io/gh/apache/incubator-uniffle/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL3V0aWxzL3V0aWwuZ28=) | | |
   | [...eploy/kubernetes/operator/pkg/utils/coordinator.go](https://codecov.io/gh/apache/incubator-uniffle/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL3V0aWxzL2Nvb3JkaW5hdG9yLmdv) | | |
   | [deploy/kubernetes/operator/pkg/utils/config.go](https://codecov.io/gh/apache/incubator-uniffle/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL3V0aWxzL2NvbmZpZy5nbw==) | | |
   | [deploy/kubernetes/operator/pkg/webhook/manager.go](https://codecov.io/gh/apache/incubator-uniffle/pull/602?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL3dlYmhvb2svbWFuYWdlci5nbw==) | | |
   | ... and [18 more](https://codecov.io/gh/apache/incubator-uniffle/pull/602?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
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: issues-unsubscribe@uniffle.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org

[GitHub] [incubator-uniffle] advancedxy commented on a diff in pull request #602: [#580] chore: speed up CI workflows

Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #602:
URL: https://github.com/apache/incubator-uniffle/pull/602#discussion_r1107084703


##########
.github/workflows/build.yml:
##########
@@ -52,20 +54,31 @@ jobs:
       java-version: '11'
 
   java-17:
+    needs: [spotbugs] # delay execution
     name: 'compile'
     uses: ./.github/workflows/sequential.yml
     with:
       maven-args: package -DskipTests
       cache-key: package
       java-version: '17'
 
-  package:
+  unit:
     uses: ./.github/workflows/parallel.yml
     with:
-      maven-args: package
+      maven-args: package -Dtest=!org.apache.uniffle.test.**
+      reports-path: "**/target/surefire-reports/*.txt"
+
+  integration:
+    uses: ./.github/workflows/parallel.yml
+    with:
+      maven-args: package -Dtest=org.apache.uniffle.test.**
       reports-path: "**/target/surefire-reports/*.txt"
 
   deploy:

Review Comment:
   After this change, I don't think deploy is a proper name.`deploy` usually means deploying actual package/service.
   
   could you change it to `resource managers` or simply `k8s`?
   



-- 
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: issues-unsubscribe@uniffle.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org

[GitHub] [incubator-uniffle] advancedxy commented on a diff in pull request #602: [#580] chore: speed up CI workflows

Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #602:
URL: https://github.com/apache/incubator-uniffle/pull/602#discussion_r1107086208


##########
.github/workflows/parallel.yml:
##########
@@ -79,7 +79,7 @@ jobs:
           mvn-${{ inputs.java-version }}-package-${{ matrix.profile }}-
           mvn-${{ inputs.java-version }}-package-
     - name: Execute `mvn ${{ inputs.maven-args }} -P${{ matrix.profile }}`
-      run: mvn -B -fae ${{ inputs.maven-args }} -P${{ matrix.profile }} | tee /tmp/maven.log
+      run: mvn -T1C -B -fae ${{ inputs.maven-args }} -P${{ matrix.profile }} | tee /tmp/maven.log

Review Comment:
   do this also implies we are running tests in parallel?
   
   There might be subtle issues when running tests in parallel, such as port collision. 



-- 
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: issues-unsubscribe@uniffle.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org

[GitHub] [incubator-uniffle] advancedxy commented on pull request #602: [#580] chore: speed up CI workflows

Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #602:
URL: https://github.com/apache/incubator-uniffle/pull/602#issuecomment-1431368809

   > UT is already excluded. Some integration tests are slow, maybe we can improve the speed of them.
   And we can also split integration tests to multiple packages, to further break down the workflow and improve parallelism.
   
   Ah, I didn't see that coming since you don't specify `-DskipIT`..
   But it seems integration tests are prefixed with test package.
   
   
   > I'm not sure, maybe some tests need and some tests don't. Let's check and split them into different packages later.
   
   Yeah.. Integration tests should be in separated packages. But let's do that later.


-- 
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: issues-unsubscribe@uniffle.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org

[GitHub] [incubator-uniffle] kaijchen merged pull request #602: [#580] chore: speed up CI workflows

Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen merged PR #602:
URL: https://github.com/apache/incubator-uniffle/pull/602


-- 
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: issues-unsubscribe@uniffle.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org