You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/02/03 07:27:14 UTC
[GitHub] [spark] Yikun opened a new pull request #31454: Skip zinc start and shutdown in aarch64 os.
Yikun opened a new pull request #31454:
URL: https://github.com/apache/spark/pull/31454
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #31454: [SPARK-34341][BUILD] Skip zinc related operations on aarch64
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #31454:
URL: https://github.com/apache/spark/pull/31454#issuecomment-785874471
> How can we test that it is running the build using zinc?
@odidev Sorry for late reply, and looks like there is some misleading content in my last reply. Actually, the scala-maven-plugin v4.0.0 already support the new version Zinc 1.x with multi-arch support rather than scalor-maven-plugin. [1] So, you can just use the scala-maven-plugin v4.X to enable the Zinc incremental compiler in aarch64 and also x86.
> If we can fix it not invasively, I think it's fine to add zinc back in aarch64 too.
@HyukjinKwon And when I do investigation on the Zinc multi-arch support, I found there is bug [SPARK-34539](https://issues.apache.org/jira/projects/SPARK/issues/SPARK-34539 ), looks like the zinc server code can be cleanup completely after we bump the scala-maven-plugin to v4.X in [SPARK-28759](https://issues.apache.org/jira/browse/SPARK-28759), because the zinc increamental compiler has been [enabled by default](https://github.com/davidB/scala-maven-plugin/commit/0f01f2fe37aaadbefb02fb713d469dbd10acf8b1)[2] in scala-maven-plugin 4.X.
[1] https://github.com/davidB/scala-maven-plugin/pull/321
[2] https://github.com/davidB/scala-maven-plugin/commit/0f01f2fe37aaadbefb02fb713d469dbd10acf8b1
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] odidev commented on pull request #31454: [SPARK-34341][BUILD] Skip zinc related operations on aarch64
Posted by GitBox <gi...@apache.org>.
odidev commented on pull request #31454:
URL: https://github.com/apache/spark/pull/31454#issuecomment-783451286
@HyukjinKwon @huangtianhua, I was actually interested in enablement of "zinc" operations in aarch64 as well. Are you planning to add this in future? @Yikun I've tried modifying the pom.xml to to add **scalor-maven-plugin** to enable zinc, but since it is getting compiled and tests are running without zinc also, How can we test that it is running the build using zinc? Let me know if i am m missing something 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #31454: [SPARK-34341][BUILD] Skip zinc related operations on aarch64
Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #31454:
URL: https://github.com/apache/spark/pull/31454#issuecomment-785874471
@odidev Sorry for late reply, and looks like I have some misleading content in my last reply. Actually, the scala-maven-plugin v4.0.0 already support the new version Zinc 1.x with multi-arch support rather than scalor-maven-plugin. [1] So, you can just use the scala-maven-plugin v4.X to enable the Zinc incremental compiler in aarch64 and also x86.
And when I do investigation on the Zinc multi-arch support, I found there is bug [SPARK-34539](https://issues.apache.org/jira/projects/SPARK/issues/SPARK-34539 ), looks like the zinc server code can be cleanup completely after we bump the scala-maven-plugin to v4.X [SPARK-28759](https://issues.apache.org/jira/browse/SPARK-28759).
[1] https://github.com/davidB/scala-maven-plugin/pull/321
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon edited a comment on pull request #31454: [SPARK-34341][BUILD] Skip zinc related operations on aarch64
Posted by GitBox <gi...@apache.org>.
HyukjinKwon edited a comment on pull request #31454:
URL: https://github.com/apache/spark/pull/31454#issuecomment-783792240
Manual test is fine (just showing the logs in the PR description). If we can fix it not invasively, I think it's fine to add zinc back in aarch64 too.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #31454: [SPARK-34341][BUILD] Skip zinc related operations on aarch64
Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #31454:
URL: https://github.com/apache/spark/pull/31454#issuecomment-772410389
@HyukjinKwon Thanks for you super fast reply. : )
> Do you have any issue link in zinc for this?
I think it's a little diffcult to get this link, because the latest standalone version zinc (0.3.15, spark used), it is already deprecated, so we have no chance to raise a issue to confirm the Arm support from zinc officially [1].
> It would be great if we can confirm that zinc is not supported in aarch64.
But I can confirmed the 0.3.15 zinc, will call `nailgun` and finally call ng, but `ng` doesn't have completely arm support. [2][3] so that I got this `cannot execute binary file: Exec format error` here.
And another info is that I check the sbt/zinc [4], looks like we can only use the scalor-maven-plugin to enable zinc with maven, but not the stand alone style. I do some try on this plugin demo[5] on aarch64, it works.
IMO, maybe the solution is that first we **skip the zinc operations on aarch64** in here , and **switch standalone zinc to `scalor-maven-plugin` to enable zinc** in future (if our spark really want).
[1] https://github.com/typesafehub/zinc
[2] https://github.com/typesafehub/zinc/blob/8522622/src/universal/bin/nailgun#L36-L43
[3] https://github.com/typesafehub/zinc/tree/8522622/src/universal/bin/ng
[4] https://github.com/sbt/zinc
[5] https://github.com/random-maven/scalor-maven-plugin/tree/master/demo
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huangtianhua commented on pull request #31454: [SPARK-34341][BUILD] Skip zinc related operations on aarch64
Posted by GitBox <gi...@apache.org>.
huangtianhua commented on pull request #31454:
URL: https://github.com/apache/spark/pull/31454#issuecomment-772955556
Sorry reply late. Yes, we found standalone zinc is not supported well on aarch64 before, but it doesn't affect mvn building/testing, so we ignored it. To skip zinc related commands on aarch64 first is ok to me, so +1 :) 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #31454: [SPARK-34341][BUILD] Skip zinc related operations on aarch64
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #31454:
URL: https://github.com/apache/spark/pull/31454#issuecomment-772441281
cc @huangtianhua, do you have any input on 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #31454: [SPARK-34341][BUILD] Skip zinc related operations on aarch64
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #31454:
URL: https://github.com/apache/spark/pull/31454#issuecomment-772370929
@Yikun,
> The standalone zinc is not supported well on aarch64, so that the error ouput, `cannot execute binary file: Exec format error` dumped after build/mvn is called.
Do you have any issue link in zinc for this? It would be great if we can confirm that zinc is not supported in aarch64.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #31454: [SPARK-34341][BUILD] Skip zinc related operations on aarch64
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #31454:
URL: https://github.com/apache/spark/pull/31454#issuecomment-785874471
> How can we test that it is running the build using zinc?
@odidev Sorry for late reply, and looks like there is some misleading content in my last reply. Actually, the scala-maven-plugin v4.0.0 already support the new version Zinc 1.x with multi-arch support rather than scalor-maven-plugin. [1] So, you can just use the scala-maven-plugin v4.X to enable the Zinc incremental compiler in aarch64 and also x86.
> If we can fix it not invasively, I think it's fine to add zinc back in aarch64 too.
@HyukjinKwon And when I do investigation on the Zinc multi-arch support, I found there is bug [SPARK-34539](https://issues.apache.org/jira/projects/SPARK/issues/SPARK-34539 ), looks like the zinc server code can be cleanup completely after we bump the scala-maven-plugin to v4.X in [SPARK-28759](https://issues.apache.org/jira/browse/SPARK-28759), because the zinc increamental compiler has been enabled by default in scala-maven-plugin 4.X.
[1] https://github.com/davidB/scala-maven-plugin/pull/321
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #31454: [SPARK-34341][BUILD] Skip zinc related operations on aarch64
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #31454:
URL: https://github.com/apache/spark/pull/31454#issuecomment-783792240
Manual test is fine. If we can fix it not invasively, I think it's fine to add zinc back in aarch64 too.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31454: [WIP] Skip zinc start and shutdown in aarch64 os.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31454:
URL: https://github.com/apache/spark/pull/31454#issuecomment-772312572
Can one of the admins verify this patch?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #31454: [SPARK-34341][BUILD] Skip zinc related operations on aarch64
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #31454:
URL: https://github.com/apache/spark/pull/31454#issuecomment-785874471
@odidev Sorry for late reply, and looks like I have some misleading content in my last reply. Actually, the scala-maven-plugin v4.0.0 already support the new version Zinc 1.x with multi-arch support rather than scalor-maven-plugin. [1] So, you can just use the scala-maven-plugin v4.X to enable the Zinc incremental compiler in aarch64 and also x86.
@HyukjinKwon And when I do investigation on the Zinc multi-arch support, I found there is bug [SPARK-34539](https://issues.apache.org/jira/projects/SPARK/issues/SPARK-34539 ), looks like the zinc server code can be cleanup completely after we bump the scala-maven-plugin to v4.X [SPARK-28759](https://issues.apache.org/jira/browse/SPARK-28759).
[1] https://github.com/davidB/scala-maven-plugin/pull/321
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon closed pull request #31454: [SPARK-34341][BUILD] Skip zinc related operations on aarch64
Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #31454:
URL: https://github.com/apache/spark/pull/31454
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #31454: [SPARK-34341][BUILD] Skip zinc related operations on aarch64
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #31454:
URL: https://github.com/apache/spark/pull/31454#issuecomment-772960037
Merged to master.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org