You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by xuanyuanking <gi...@git.apache.org> on 2018/11/09 05:26:16 UTC
[GitHub] spark pull request #22989: [SPARK-25986][Build] Banning throw new OutOfMemor...
GitHub user xuanyuanking opened a pull request:
https://github.com/apache/spark/pull/22989
[SPARK-25986][Build] Banning throw new OutOfMemoryErrors
## What changes were proposed in this pull request?
Add scala and java lint check rules to ban the usage of `throw new OutOfMemoryErrors` cause it will cause hole executor killed. See more details in https://github.com/apache/spark/pull/22969.
## How was this patch tested?
Local test with lint-scala and lint-java.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/xuanyuanking/spark SPARK-25986
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22989.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #22989
----
commit d67875115f622082519b1dbcb1c1e34c2184b34f
Author: Yuanjian Li <xy...@...>
Date: 2018-11-09T05:23:01Z
banning throw new OutOfMemoryErrors
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Banning throw new OutOfMemor...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r232245886
--- Diff: scalastyle-config.xml ---
@@ -240,6 +240,18 @@ This file is divided into 3 sections:
]]></customMessage>
</check>
+ <check customId="notthrowoutofmemory" level="error" class="org.scalastyle.file.RegexChecker" enabled="true">
+ <parameters><parameter name="regex">throw new OutOfMemoryError</parameter></parameters>
+ <customMessage><![CDATA[
+ Are you sure that you want to throw OutOfMemoryError? It will kill the hole executor, you should use
--- End diff --
hole -> whole or entire
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4877/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Add rules to ban throw Errors in ap...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98826/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Banning throw new OutOfMemor...
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r232722184
--- Diff: scalastyle-config.xml ---
@@ -240,6 +240,18 @@ This file is divided into 3 sections:
]]></customMessage>
</check>
+ <check customId="notthrowoutofmemory" level="error" class="org.scalastyle.file.RegexChecker" enabled="true">
--- End diff --
Thanks, new rule named `throwerror`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Banning throw new OutOfMemor...
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r232722383
--- Diff: dev/checkstyle.xml ---
@@ -64,6 +64,11 @@
<property name="message" value="No trailing whitespace allowed."/>
</module>
+ <module name="RegexpSingleline">
+ <property name="format" value="throw new OutOfMemoryError"/>
--- End diff --
Thanks, done in 6e49bb8.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Add rules to ban throw Errors in ap...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22989
**[Test build #98770 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98770/testReport)** for PR 22989 at commit [`a4f49ce`](https://github.com/apache/spark/commit/a4f49ceae55181e3157852ade5bd26f5f5248c80).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Banning throw new OutOfMemor...
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r232721829
--- Diff: dev/checkstyle.xml ---
@@ -64,6 +64,11 @@
<property name="message" value="No trailing whitespace allowed."/>
</module>
+ <module name="RegexpSingleline">
+ <property name="format" value="throw new OutOfMemoryError"/>
+ <property name="message" value="Throw OutOfMemoryError will kill entire executor, replace it with SparkOutOfMemoryError."/>
--- End diff --
Thanks, rewrite this rule in 6e49bb8.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Add rules to ban throw Error...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r233706517
--- Diff: common/unsafe/src/main/java/org/apache/spark/unsafe/UnsafeAlignedOffset.java ---
@@ -39,7 +39,9 @@ public static int getSize(Object object, long offset) {
case 8:
return (int)Platform.getLong(object, offset);
default:
+ // checkstyle.off: RegexpSinglelineJava
throw new AssertionError("Illegal UAO_SIZE");
--- End diff --
shall we throw `IllegalStateException` here?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Add rules to ban throw Error...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r233318346
--- Diff: dev/checkstyle.xml ---
@@ -180,5 +180,10 @@
<module name="UnusedImports"/>
<module name="RedundantImport"/>
<module name="RedundantModifier"/>
+ <module name="RegexpSinglelineJava">
+ <property name="format" value="throw new \w+Error\("/>
+ <property name="message" value="Avoid throwing error in application code."/>
--- End diff --
What `application code` means here?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Add rules to ban throw Errors in ap...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22989
**[Test build #98770 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98770/testReport)** for PR 22989 at commit [`a4f49ce`](https://github.com/apache/spark/commit/a4f49ceae55181e3157852ade5bd26f5f5248c80).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4886/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22989
**[Test build #98649 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98649/testReport)** for PR 22989 at commit [`d678751`](https://github.com/apache/spark/commit/d67875115f622082519b1dbcb1c1e34c2184b34f).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Banning throw new OutOfMemor...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r232493703
--- Diff: scalastyle-config.xml ---
@@ -240,6 +240,18 @@ This file is divided into 3 sections:
]]></customMessage>
</check>
+ <check customId="notthrowoutofmemory" level="error" class="org.scalastyle.file.RegexChecker" enabled="true">
+ <parameters><parameter name="regex">throw new OutOfMemoryError</parameter></parameters>
+ <customMessage><![CDATA[
+ Are you sure that you want to throw OutOfMemoryError? It will kill the hole executor, you should use
--- End diff --
yea, maybe just kill the executor or kill the current executor.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22989
`AssertionError` in tests is wrong. It should really just call `fail()`. If you're willing to fix that, it would be a great cleanup.
The `AssertionError` in UnsafeAlignedOffset is legitimate and can be excluded.
I would fix the `AssertionError` in KafkaUtils. That one is clearly an illegal argument check.
Agree about the rest. Thanks for the great analysis @xuanyuanking
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Add rules to ban throw Error...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r233713175
--- Diff: common/unsafe/src/main/java/org/apache/spark/unsafe/UnsafeAlignedOffset.java ---
@@ -39,7 +39,9 @@ public static int getSize(Object object, long offset) {
case 8:
return (int)Platform.getLong(object, offset);
default:
+ // checkstyle.off: RegexpSinglelineJava
throw new AssertionError("Illegal UAO_SIZE");
--- End diff --
I think these are ok as AssertionError because they shouldn't be able to happen in any JVM state
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Add rules to ban throw Errors in ap...
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking commented on the issue:
https://github.com/apache/spark/pull/22989
Thanks @HyukjinKwon @viirya @felixcheung @srowen for your review and advise!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Add rules to ban throw Errors in ap...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22989
Merged to master
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Add rules to ban throw Errors in ap...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22989
**[Test build #98826 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98826/testReport)** for PR 22989 at commit [`210d942`](https://github.com/apache/spark/commit/210d9420528e8c5c9b08ba54cb03797c15af94b9).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Add rules to ban throw Error...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r233821654
--- Diff: common/unsafe/src/main/java/org/apache/spark/unsafe/UnsafeAlignedOffset.java ---
@@ -39,7 +39,9 @@ public static int getSize(Object object, long offset) {
case 8:
return (int)Platform.getLong(object, offset);
default:
+ // checkstyle.off: RegexpSinglelineJava
throw new AssertionError("Illegal UAO_SIZE");
--- End diff --
yea, that's exactly the use case of `IllegalStateException`, which can also pass the style check here.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Add rules to ban throw Errors in ap...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking commented on the issue:
https://github.com/apache/spark/pull/22989
Sorry for late reply, great thanks for all reviewer's advise, will address them soon.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking commented on the issue:
https://github.com/apache/spark/pull/22989
cc all reviewer, as @srowen's suggestion, add a rule to ban all of new Error cases.
List currently `throw new XXXError` in Spark source below and record fix up or exclude for review conveniently:
ErrorName | Count | In Test/Source Code | Fix Up or Exclude
------------ | ------------- | ------------- | -------------
AssertionError | 32 | 29 in test code | Exclude
| | | 2 in UnsafeAlignedOffset | Fix up by changing them to `IllegalArgumentException`
| | | 1 in KafkaUtils | Just exclude cause fix it will cause behavior change
NotImplementedError | 22 | 7 in source code/ 15 in test cases | Fix up by changing them to `UnsupportedOperationException`
OutOfMemoryError | 1 | 1 in test code | Exclude
LinkageError | 1 | 1 in test code | Exclude
SparkOutOfMemoryError | 5 | 5 in source code | Exclude
UnknownError | 9 | 5 in source code/ 4 in test cases | Fix up by changing to `IllegalAccessException ` and `IllegalArgumentException`
All above done in 6e49bb8.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Add rules to ban throw Error...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r232917777
--- Diff: dev/checkstyle-suppressions.xml ---
@@ -46,4 +46,12 @@
files="sql/catalyst/src/main/java/org/apache/spark/sql/streaming/GroupStateTimeout.java"/>
<suppress checks="MethodName"
files="sql/core/src/main/java/org/apache/spark/sql/streaming/Trigger.java"/>
+ <suppress checks="RegexpSingleline"
--- End diff --
I think this might suppress many checks? is it difficult to suppress with comments in these files?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22989
**[Test build #98637 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98637/testReport)** for PR 22989 at commit [`d678751`](https://github.com/apache/spark/commit/d67875115f622082519b1dbcb1c1e34c2184b34f).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4962/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Add rules to ban throw Error...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/22989
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Add rules to ban throw Errors in ap...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98749/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Add rules to ban throw Errors in ap...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4980/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22989
**[Test build #98731 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98731/testReport)** for PR 22989 at commit [`6e49bb8`](https://github.com/apache/spark/commit/6e49bb8cca201237a7871b22f178609319a0fdce).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22989
**[Test build #98731 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98731/testReport)** for PR 22989 at commit [`6e49bb8`](https://github.com/apache/spark/commit/6e49bb8cca201237a7871b22f178609319a0fdce).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Add rules to ban throw Error...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r232917995
--- Diff: mllib/src/test/scala/org/apache/spark/ml/feature/VectorIndexerSuite.scala ---
@@ -283,7 +283,9 @@ class VectorIndexerSuite extends MLTest with DefaultReadWriteTest with Logging {
points.zip(rows.map(_(0))).foreach {
case (orig: SparseVector, indexed: SparseVector) =>
assert(orig.indices.length == indexed.indices.length)
- case _ => throw new UnknownError("Unit test has a bug in it.") // should never happen
+ case _ =>
+ // should never happen
+ throw new IllegalAccessException("Unit test has a bug in it.")
--- End diff --
Just `fail()` here? or at least not `IllegalAccessException`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22989
**[Test build #98649 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98649/testReport)** for PR 22989 at commit [`d678751`](https://github.com/apache/spark/commit/d67875115f622082519b1dbcb1c1e34c2184b34f).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Banning throw new OutOfMemor...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r232271235
--- Diff: dev/checkstyle.xml ---
@@ -64,6 +64,11 @@
<property name="message" value="No trailing whitespace allowed."/>
</module>
+ <module name="RegexpSingleline">
+ <property name="format" value="throw new OutOfMemoryError"/>
+ <property name="message" value="Throw OutOfMemoryError will kill entire executor, replace it with SparkOutOfMemoryError."/>
--- End diff --
Just say that OutOfMemoryError should not generally be thrown by application code. It may not actually be replaceable by SparkOutOfMemoryError
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Banning throw new OutOfMemor...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r232477783
--- Diff: scalastyle-config.xml ---
@@ -240,6 +240,18 @@ This file is divided into 3 sections:
]]></customMessage>
</check>
+ <check customId="notthrowoutofmemory" level="error" class="org.scalastyle.file.RegexChecker" enabled="true">
+ <parameters><parameter name="regex">throw new OutOfMemoryError</parameter></parameters>
+ <customMessage><![CDATA[
+ Are you sure that you want to throw OutOfMemoryError? It will kill the hole executor, you should use
--- End diff --
perhasp "it will kill the whole executor" is a bit hard to understand? (I mean, is there a way to kill part of the executor?)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/22989
and catching Error or Throwable..
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Add rules to ban throw Errors in ap...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Add rules to ban throw Error...
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r232984147
--- Diff: mllib/src/test/scala/org/apache/spark/ml/feature/VectorIndexerSuite.scala ---
@@ -283,7 +283,9 @@ class VectorIndexerSuite extends MLTest with DefaultReadWriteTest with Logging {
points.zip(rows.map(_(0))).foreach {
case (orig: SparseVector, indexed: SparseVector) =>
assert(orig.indices.length == indexed.indices.length)
- case _ => throw new UnknownError("Unit test has a bug in it.") // should never happen
+ case _ =>
+ // should never happen
+ throw new IllegalAccessException("Unit test has a bug in it.")
--- End diff --
Thanks, done in a4f49ce by just `fail` here.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Add rules to ban throw Errors in ap...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22989
**[Test build #98749 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98749/testReport)** for PR 22989 at commit [`ff234d3`](https://github.com/apache/spark/commit/ff234d31a5a8e296b845910717dcd78be67b1740).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Add rules to ban throw Error...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r233317742
--- Diff: mllib/src/test/scala/org/apache/spark/mllib/clustering/KMeansSuite.scala ---
@@ -331,7 +333,7 @@ class KMeansSuite extends SparkFunSuite with MLlibTestSparkContext {
}
}
-object KMeansSuite extends SparkFunSuite {
+object KMeansSuite extends SparkFunSuite with Assertions {
--- End diff --
Isn't `Assertions` redundant?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Add rules to ban throw Error...
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r232955017
--- Diff: dev/checkstyle-suppressions.xml ---
@@ -46,4 +46,12 @@
files="sql/catalyst/src/main/java/org/apache/spark/sql/streaming/GroupStateTimeout.java"/>
<suppress checks="MethodName"
files="sql/core/src/main/java/org/apache/spark/sql/streaming/Trigger.java"/>
+ <suppress checks="RegexpSingleline"
--- End diff --
Yep, I met some problem during suppress with comments, just work around by suppress files, I will find the problem again.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Add rules to ban throw Errors in ap...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Add rules to ban throw Errors in ap...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Banning throw new OutOfMemor...
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r232721412
--- Diff: scalastyle-config.xml ---
@@ -240,6 +240,18 @@ This file is divided into 3 sections:
]]></customMessage>
</check>
+ <check customId="notthrowoutofmemory" level="error" class="org.scalastyle.file.RegexChecker" enabled="true">
+ <parameters><parameter name="regex">throw new OutOfMemoryError</parameter></parameters>
+ <customMessage><![CDATA[
+ Are you sure that you want to throw OutOfMemoryError? It will kill the hole executor, you should use
--- End diff --
Rewrite this rule for banning all throw Error in 6e49bb8.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Add rules to ban throw Error...
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r233432568
--- Diff: dev/checkstyle.xml ---
@@ -180,5 +180,10 @@
<module name="UnusedImports"/>
<module name="RedundantImport"/>
<module name="RedundantModifier"/>
+ <module name="RegexpSinglelineJava">
+ <property name="format" value="throw new \w+Error\("/>
+ <property name="message" value="Avoid throwing error in application code."/>
--- End diff --
`application code` against JVM here, as Error subclasses generally represent internal JVM errors.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Add rules to ban throw Errors in ap...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5024/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Add rules to ban throw Errors in ap...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22989
**[Test build #98826 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98826/testReport)** for PR 22989 at commit [`210d942`](https://github.com/apache/spark/commit/210d9420528e8c5c9b08ba54cb03797c15af94b9).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Add rules to ban throw Error...
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r233432630
--- Diff: mllib/src/test/scala/org/apache/spark/mllib/clustering/KMeansSuite.scala ---
@@ -331,7 +333,7 @@ class KMeansSuite extends SparkFunSuite with MLlibTestSparkContext {
}
}
-object KMeansSuite extends SparkFunSuite {
+object KMeansSuite extends SparkFunSuite with Assertions {
--- End diff --
ah thanks! Done in 210d942.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Add rules to ban throw Error...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r233706605
--- Diff: common/unsafe/src/main/java/org/apache/spark/unsafe/UnsafeAlignedOffset.java ---
@@ -52,7 +54,9 @@ public static void putSize(Object object, long offset, int value) {
Platform.putLong(object, offset, value);
break;
default:
+ // checkstyle.off: RegexpSinglelineJava
throw new AssertionError("Illegal UAO_SIZE");
--- End diff --
ditto
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Add rules to ban throw Error...
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r232983941
--- Diff: dev/checkstyle-suppressions.xml ---
@@ -46,4 +46,12 @@
files="sql/catalyst/src/main/java/org/apache/spark/sql/streaming/GroupStateTimeout.java"/>
<suppress checks="MethodName"
files="sql/core/src/main/java/org/apache/spark/sql/streaming/Trigger.java"/>
+ <suppress checks="RegexpSingleline"
--- End diff --
I found the problem is `RegexpSingleline` can't put into `TreeWalker`, so `SuppressionCommentFilter` rule can't effect on it, `RegexpSinglelineJava` is the right choice. Also spend some time on comment syntax, maybe the usage description can be improved a little, please have a check whether I understand this rule correctly. Done in a4f49ce.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Banning throw new OutOfMemor...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r232289280
--- Diff: scalastyle-config.xml ---
@@ -240,6 +240,18 @@ This file is divided into 3 sections:
]]></customMessage>
</check>
+ <check customId="notthrowoutofmemory" level="error" class="org.scalastyle.file.RegexChecker" enabled="true">
--- End diff --
not a big deal but I would name it `nothrowoutofmemory`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22989
Honestly it is bad style to throw any Error class, except in sepcial situations. I'd prefer a rule banning all of these and fix up any instances in the code or else exclude them
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Add rules to ban throw Errors in ap...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22989
**[Test build #98749 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98749/testReport)** for PR 22989 at commit [`ff234d3`](https://github.com/apache/spark/commit/ff234d31a5a8e296b845910717dcd78be67b1740).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Banning throw new OutOfMemor...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r232496215
--- Diff: scalastyle-config.xml ---
@@ -240,6 +240,18 @@ This file is divided into 3 sections:
]]></customMessage>
</check>
+ <check customId="notthrowoutofmemory" level="error" class="org.scalastyle.file.RegexChecker" enabled="true">
+ <parameters><parameter name="regex">throw new OutOfMemoryError</parameter></parameters>
+ <customMessage><![CDATA[
+ Are you sure that you want to throw OutOfMemoryError? It will kill the hole executor, you should use
--- End diff --
This rule wouldn't be specific to executor code though. Just say it's bad practice to throw Error
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Banning throw new OutOfMemor...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r232491342
--- Diff: dev/checkstyle.xml ---
@@ -64,6 +64,11 @@
<property name="message" value="No trailing whitespace allowed."/>
</module>
+ <module name="RegexpSingleline">
+ <property name="format" value="throw new OutOfMemoryError"/>
--- End diff --
Per my suggestion below, how about the regex `throw new \\w+Error\\(`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking commented on the issue:
https://github.com/apache/spark/pull/22989
retest this please.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Add rules to ban throw Errors in ap...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98637/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Add rules to ban throw Errors in ap...
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking commented on the issue:
https://github.com/apache/spark/pull/22989
@srowen Great thanks for your guidance, address all your suggestion in ff234d3 and update the record table in https://github.com/apache/spark/pull/22989#issuecomment-437939830.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Add rules to ban throw Errors in ap...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98770/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98649/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22989: [SPARK-25986][Build] Add rules to ban throw Error...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22989#discussion_r233842829
--- Diff: common/unsafe/src/main/java/org/apache/spark/unsafe/UnsafeAlignedOffset.java ---
@@ -39,7 +39,9 @@ public static int getSize(Object object, long offset) {
case 8:
return (int)Platform.getLong(object, offset);
default:
+ // checkstyle.off: RegexpSinglelineJava
throw new AssertionError("Illegal UAO_SIZE");
--- End diff --
It would be OK too. This is just picking nits now but I thought AssertionError was still better. ISE is about application state. Errors are about JVM state.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22989
**[Test build #98637 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98637/testReport)** for PR 22989 at commit [`d678751`](https://github.com/apache/spark/commit/d67875115f622082519b1dbcb1c1e34c2184b34f).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98731/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22989: [SPARK-25986][Build] Banning throw new OutOfMemoryErrors
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22989
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4952/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org