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 2022/11/10 02:17:23 UTC
[GitHub] [spark] LuciferYang opened a new pull request, #38589: [SPARK-41087][BUILD] Make `build/mvn` use the same JAVA_OPTS as `dev/make-distribution.sh`
LuciferYang opened a new pull request, #38589:
URL: https://github.com/apache/spark/pull/38589
### 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.
If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
-->
--
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: reviews-unsubscribe@spark.apache.org
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] bjornjorgensen commented on pull request #38589: [SPARK-41087][BUILD] Remove duplicate `-Xmx4g` from `dev/make-distribution.sh` and make `build/mvn` use the same JAVA_OPTS
Posted by GitBox <gi...@apache.org>.
bjornjorgensen commented on PR #38589:
URL: https://github.com/apache/spark/pull/38589#issuecomment-1310115370
Tests are failing because of this. https://github.com/apache/spark/commit/c2a8e48e70abfb6bd101c99c5a0f6017151fc85e
--
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: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on pull request #38589: [SPARK-41087][BUILD] Remove duplicate `-Xmx4g` from `dev/make-distribution.sh` and make `build/mvn` use the same JAVA_OPTS
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #38589:
URL: https://github.com/apache/spark/pull/38589#issuecomment-1313153448
The test is divided into two parts:
1. Bare metal server, `Intel(R) Xeon(R) Gold 6271C CPU @ 2.60GHz` with Java 8u352, test command as follows:
```
build/mvn clean install -Phadoop-3 -Phadoop-cloud -Pmesos -Pyarn -Pkinesis-asl -Phive-thriftserver -Pspark-ganglia-lgpl -Pkubernetes -Phive -fn
```
All test passed, but in this part , I skip the `connector/connect` and `connector/protobuf` modules because they can't compiled on the centos7u4 OS now（I'm trying to solve this issue）
2.Mac OS/Apple Silicon, M1 with Java 17.0.5, test command as follows:
```
build/mvn clean install -Phadoop-3 -Phadoop-cloud -Pmesos -Pyarn -Pkinesis-asl -Phive-thriftserver -Pspark-ganglia-lgpl -Pkubernetes -Phive -fn -Dtest.exclude.tags=org.apache.spark.tags.ExtendedLevelDBTest
```
--
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: reviews-unsubscribe@spark.apache.org
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] dongjoon-hyun commented on pull request #38589: [SPARK-41087][BUILD] Make `build/mvn` use the same JAVA_OPTS as `dev/make-distribution.sh`
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #38589:
URL: https://github.com/apache/spark/pull/38589#issuecomment-1309846227
Could you revise the PR title according to the new change?
--
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: reviews-unsubscribe@spark.apache.org
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] dongjoon-hyun commented on a diff in pull request #38589: [SPARK-41087][BUILD] Make `build/mvn` use the same JAVA_OPTS as `dev/make-distribution.sh`
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #38589:
URL: https://github.com/apache/spark/pull/38589#discussion_r1018688674
##########
build/mvn:
##########
@@ -36,7 +36,7 @@ _DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
# Preserve the calling directory
_CALLING_DIR="$(pwd)"
# Options used during compilation
-_COMPILE_JVM_OPTS="-Xmx2g -XX:ReservedCodeCacheSize=1g -Xss128m"
+_COMPILE_JVM_OPTS="-Xss128m -Xmx4g -Xmx4g -XX:ReservedCodeCacheSize=128m"
Review Comment:
May I ask why you repeat `-Xmx4g` twice?
--
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: reviews-unsubscribe@spark.apache.org
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] dongjoon-hyun closed pull request #38589: [SPARK-41087][BUILD] Remove duplicated `-Xmx4g` from `dev/make-distribution.sh` and make `build/mvn` use the same JAVA_OPTS
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #38589: [SPARK-41087][BUILD] Remove duplicated `-Xmx4g` from `dev/make-distribution.sh` and make `build/mvn` use the same JAVA_OPTS
URL: https://github.com/apache/spark/pull/38589
--
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: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on a diff in pull request #38589: [SPARK-41087][BUILD] Make `build/mvn` use the same JAVA_OPTS as `dev/make-distribution.sh`
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on code in PR #38589:
URL: https://github.com/apache/spark/pull/38589#discussion_r1018691938
##########
build/mvn:
##########
@@ -36,7 +36,7 @@ _DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
# Preserve the calling directory
_CALLING_DIR="$(pwd)"
# Options used during compilation
-_COMPILE_JVM_OPTS="-Xmx2g -XX:ReservedCodeCacheSize=1g -Xss128m"
+_COMPILE_JVM_OPTS="-Xss128m -Xmx4g -Xmx4g -XX:ReservedCodeCacheSize=128m"
Review Comment:
Or maybe we can change it to `-Xms4g`
--
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: reviews-unsubscribe@spark.apache.org
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] dongjoon-hyun commented on pull request #38589: [SPARK-41087][BUILD] Remove duplicate `-Xmx4g` from `dev/make-distribution.sh` and make `build/mvn` use the same JAVA_OPTS
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #38589:
URL: https://github.com/apache/spark/pull/38589#issuecomment-1313078598
BTW, @LuciferYang . Did you run the full Maven testing with this configuration? `dev/make-distribution.sh` is simply for building while `build/mvn` should support testing. Please run a full testing to verify your PR doesn't break anything in Maven testing.
--
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: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on pull request #38589: [SPARK-41087][BUILD] Make `build/mvn` use the same JAVA_OPTS as `dev/make-distribution.sh`
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #38589:
URL: https://github.com/apache/spark/pull/38589#issuecomment-1309822675
Wait me check maven test all modules
--
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: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on a diff in pull request #38589: [SPARK-41087][BUILD] Make `build/mvn` use the same JAVA_OPTS as `dev/make-distribution.sh`
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on code in PR #38589:
URL: https://github.com/apache/spark/pull/38589#discussion_r1018690310
##########
build/mvn:
##########
@@ -36,7 +36,7 @@ _DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
# Preserve the calling directory
_CALLING_DIR="$(pwd)"
# Options used during compilation
-_COMPILE_JVM_OPTS="-Xmx2g -XX:ReservedCodeCacheSize=1g -Xss128m"
+_COMPILE_JVM_OPTS="-Xss128m -Xmx4g -Xmx4g -XX:ReservedCodeCacheSize=128m"
Review Comment:
Sorry, it copy from
https://github.com/apache/spark/blob/865a3ded2ea1ca86be93df58205882bc509b98cd/dev/make-distribution.sh#L164
do you mind also remove the dup `-Xmx4g` from `make-distribution.sh` in this pr?
--
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: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on a diff in pull request #38589: [SPARK-41087][BUILD] Make `build/mvn` use the same JAVA_OPTS as `dev/make-distribution.sh`
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on code in PR #38589:
URL: https://github.com/apache/spark/pull/38589#discussion_r1018700848
##########
build/mvn:
##########
@@ -36,7 +36,7 @@ _DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
# Preserve the calling directory
_CALLING_DIR="$(pwd)"
# Options used during compilation
-_COMPILE_JVM_OPTS="-Xmx2g -XX:ReservedCodeCacheSize=1g -Xss128m"
+_COMPILE_JVM_OPTS="-Xss128m -Xmx4g -Xmx4g -XX:ReservedCodeCacheSize=128m"
Review Comment:
Adding `-Xms4g` will be faster, but users do not necessarily need this option, so I choose to remove the duplicate `-Xmx4g`
--
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: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on pull request #38589: [SPARK-41087][BUILD] Remove duplicate `-Xmx4g` from `dev/make-distribution.sh` and make `build/mvn` use the same JAVA_OPTS
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #38589:
URL: https://github.com/apache/spark/pull/38589#issuecomment-1310450309
maven test all passed
--
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: reviews-unsubscribe@spark.apache.org
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] dongjoon-hyun commented on pull request #38589: [SPARK-41087][BUILD] Remove duplicate `-Xmx4g` from `dev/make-distribution.sh` and make `build/mvn` use the same JAVA_OPTS
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #38589:
URL: https://github.com/apache/spark/pull/38589#issuecomment-1313135223
If you verified it, it will be okay. So, could you describe your 4-days-ago test environment a little more to us, @LuciferYang ?
--
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: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on pull request #38589: [SPARK-41087][BUILD] Make `build/mvn` use the same JAVA_OPTS as `dev/make-distribution.sh`
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #38589:
URL: https://github.com/apache/spark/pull/38589#issuecomment-1309821310
cc @dongjoon-hyun is this one ok?
--
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: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on pull request #38589: [SPARK-41087][BUILD] Remove duplicate `-Xmx4g` from `dev/make-distribution.sh` and make `build/mvn` use the same JAVA_OPTS
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #38589:
URL: https://github.com/apache/spark/pull/38589#issuecomment-1309849459
done
--
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: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on pull request #38589: [SPARK-41087][BUILD] Remove duplicate `-Xmx4g` from `dev/make-distribution.sh` and make `build/mvn` use the same JAVA_OPTS
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #38589:
URL: https://github.com/apache/spark/pull/38589#issuecomment-1310243986
rebased, thanks @bjornjorgensen
--
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: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on a diff in pull request #38589: [SPARK-41087][BUILD] Make `build/mvn` use the same JAVA_OPTS as `dev/make-distribution.sh`
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on code in PR #38589:
URL: https://github.com/apache/spark/pull/38589#discussion_r1018700848
##########
build/mvn:
##########
@@ -36,7 +36,7 @@ _DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
# Preserve the calling directory
_CALLING_DIR="$(pwd)"
# Options used during compilation
-_COMPILE_JVM_OPTS="-Xmx2g -XX:ReservedCodeCacheSize=1g -Xss128m"
+_COMPILE_JVM_OPTS="-Xss128m -Xmx4g -Xmx4g -XX:ReservedCodeCacheSize=128m"
Review Comment:
removed the duplicate `-Xmx4g`
--
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: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on pull request #38589: [SPARK-41087][BUILD] Remove duplicated `-Xmx4g` from `dev/make-distribution.sh` and make `build/mvn` use the same JAVA_OPTS
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #38589:
URL: https://github.com/apache/spark/pull/38589#issuecomment-1313170801
Thanks @dongjoon-hyun, as mentioned earlier, I will further find for a more suitable `ReservedCodeCacheSize ` to make the build test process more stable and fast
--
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: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on pull request #38589: [SPARK-41087][BUILD] Remove duplicate `-Xmx4g` from `dev/make-distribution.sh` and make `build/mvn` use the same JAVA_OPTS
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #38589:
URL: https://github.com/apache/spark/pull/38589#issuecomment-1313122325
> maven test all passed
> Did you run the full Maven testing with this configuration?
Yes. 4 days ago, I checked the full UTs with this configuration and all test passed.
For `-XX:ReservedCodeCacheSize=128m`, there will be still `CodeCache is full. Compiler has been disabled.` related warnings during the full test, but there is no test failed. I am investigating a suitable value to unify this option, and want to change other places, such as `scala-maven-plugin`, `maven-surefire-plugin`, and `scalatest-maven-plugin`.
@dongjoon-hyun Are you sure `-XX:ReservedCodeCacheSize=1g` is a more suitable option? If confirmed, I can use `-XX:ReservedCodeCacheSize=1g` for compilation and full UTs check again, and then change the option in other places to `-XX:ReservedCodeCacheSize=1g` if it is better
--
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: reviews-unsubscribe@spark.apache.org
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