You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by a-roberts <gi...@git.apache.org> on 2017/02/10 16:35:28 UTC
[GitHub] spark pull request #16888: [SPARK-19552] [BUILD] Upgrade Netty version to 4....
GitHub user a-roberts opened a pull request:
https://github.com/apache/spark/pull/16888
[SPARK-19552] [BUILD] Upgrade Netty version to 4.1.8 final
## What changes were proposed in this pull request?
We should use Netty 4.1.8 due to one security concern and other bug fixes - we'll need to change Spark code as well as the usual misc files (like pom.xml and deps files) to enable this
## How was this patch tested?
Existing unit tests - this is a WIP as this introduces some new test failures and I have dummy implementations for the required methods I've added.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/a-roberts/spark NettyUpgrade418Final
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/16888.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 #16888
----
commit 5ace15b8c8abba562c0abbf598464014d09ad4d5
Author: Adam Roberts <ar...@uk.ibm.com>
Date: 2017-02-09T17:07:59Z
Use netty-all 4.1.8 final
commit dc88508644102ba241d75416a56afaa695db0f45
Author: Adam Roberts <ar...@uk.ibm.com>
Date: 2017-02-10T16:08:38Z
Tidy up
commit cc4fb3775eafeeb5071115e1bb988a565d94295d
Author: Adam Roberts <ar...@uk.ibm.com>
Date: 2017-02-10T16:10:44Z
Upgrade deps files as well
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16888: [SPARK-19552] [BUILD] Upgrade Netty version to 4.1.8 fin...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/16888
BTW for Netty we shouldn't just bump to the highest version. We should use the maintenance branches.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16888: [SPARK-19552] [BUILD] Upgrade Netty version to 4.1.8 fin...
Posted by a-roberts <gi...@git.apache.org>.
Github user a-roberts commented on the issue:
https://github.com/apache/spark/pull/16888
Netty 4.1.1 and above has the fix in (a change to OpenSslEngine)- Netty 4.0.x does not, and the implementations of OpenSslEngine differs substantially between the releases (so a simple case of adding in the fix would be non-trivial and potentially lead to complications, this would be the path of least resistance)
You can find more info on the CVE I'm concerned with at: https://www.versioneye.com/java/io.netty:netty-all/4.0.43.Final
To be impacted it sounds as though we need to do several things - use Spark with Netty 4.1.0 and below (including the 4.0.x releases), have tcnative on our classpath and we specify to use the OpenSslEngine (there's a useful overview on why you'd want to do this [here](https://youtu.be/DKJ0w30M0vg?t=2969) which mentions it being a drop-in replacement for the JDK classes and offers superior performance). Info on using tcnative [here](http://netty.io/wiki/forked-tomcat-native.html#wiki-h2-4)
We don't include any of the netty/tcnative native libraries for Spark so I don't think we're impacted - but moving up to take on other fixes as well would be useful (and if it so much faster we could see shuffle-intensive workloads speedup by upgrading and including natives later on). I'll set this as a WIP and look into why the tests fail.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16888: [SPARK-19552] [BUILD] Upgrade Netty version to 4.1.8 fin...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16888
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16888: [WIP] [SPARK-19552] [BUILD] Upgrade Netty version...
Posted by jpinkul <gi...@git.apache.org>.
Github user jpinkul commented on a diff in the pull request:
https://github.com/apache/spark/pull/16888#discussion_r127502197
--- Diff: common/network-common/src/main/java/org/apache/spark/network/protocol/MessageWithHeader.java ---
@@ -44,6 +44,27 @@
private final long bodyLength;
private long totalBytesTransferred;
+ @Override
+ public FileRegion touch() {
+ return this;
+ }
+
+ @Override
+ public FileRegion touch(Object hint) {
+ return this;
+ }
+
+ @Override
+ public FileRegion retain() {
+ return this;
--- End diff --
I believe this breaks the netty reference counting logic, I think instead it should be
```
return (TestFileRegion)super.retain();
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16888: [WIP] [SPARK-19552] [BUILD] Upgrade Netty version to 4.1...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/16888
OK, so there isn't actually a security case here (might update the JIRA accordingly). Updating to 4.1.x seems like a fine thing to try. If you can get it to work well, great.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16888: [SPARK-19552] [BUILD] Upgrade Netty version to 4.1.8 fin...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/16888
Updating to 4.0.44 should be fine (though, we got bit even by a maintenance update behavior change a while ago) but apparently that doesn't include the security fix in question. That said, it's not clear what the security issue is or whether it affects Spark -- can we start with that?
Updating to 4.1.x is probably a good idea, eventually, even if it's not necessary, but it will require some care and testing. It sounds like this should be marked as `[WIP]` and doesn't work yet. If it's troublesome we may not proceed with this.
CC @zsxwing
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16888: [SPARK-19552] [BUILD] Upgrade Netty version to 4.1.8 fin...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16888
**[Test build #72715 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72715/testReport)** for PR 16888 at commit [`cc4fb37`](https://github.com/apache/spark/commit/cc4fb3775eafeeb5071115e1bb988a565d94295d).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16888: [SPARK-19552] [BUILD] Upgrade Netty version to 4.1.8 fin...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16888
**[Test build #72715 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72715/testReport)** for PR 16888 at commit [`cc4fb37`](https://github.com/apache/spark/commit/cc4fb3775eafeeb5071115e1bb988a565d94295d).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16888: [SPARK-19552] [BUILD] Upgrade Netty version to 4.1.8 fin...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/16888
Shouldn't we use netty-4.0.44.Final rather than 4.1.x?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16888: [WIP] [SPARK-19552] [BUILD] Upgrade Netty version...
Posted by a-roberts <gi...@git.apache.org>.
Github user a-roberts closed the pull request at:
https://github.com/apache/spark/pull/16888
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16888: [WIP] [SPARK-19552] [BUILD] Upgrade Netty version to 4.1...
Posted by a-roberts <gi...@git.apache.org>.
Github user a-roberts commented on the issue:
https://github.com/apache/spark/pull/16888
I'll close this one then until there ever is a need to upgrade, cheers
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16888: [SPARK-19552] [BUILD] Upgrade Netty version to 4.1.8 fin...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16888
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72715/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16888: [WIP] [SPARK-19552] [BUILD] Upgrade Netty version...
Posted by jpinkul <gi...@git.apache.org>.
Github user jpinkul commented on a diff in the pull request:
https://github.com/apache/spark/pull/16888#discussion_r127502245
--- Diff: common/network-common/src/main/java/org/apache/spark/network/protocol/MessageWithHeader.java ---
@@ -44,6 +44,27 @@
private final long bodyLength;
private long totalBytesTransferred;
+ @Override
+ public FileRegion touch() {
+ return this;
+ }
+
+ @Override
+ public FileRegion touch(Object hint) {
+ return this;
+ }
+
+ @Override
+ public FileRegion retain() {
+ return this;
+ }
+
+ @Override
+ public FileRegion retain(int increment) {
+ return this;
--- End diff --
Same as above, I think this should be
```
return (TestFileRegion)super.retain(increment);
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16888: [WIP] [SPARK-19552] [BUILD] Upgrade Netty version to 4.1...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/16888
Agree, this can be closed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16888: [WIP] [SPARK-19552] [BUILD] Upgrade Netty version to 4.1...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/16888
Are there specific benefits brought by updating to 4.1 of Netty? Netty is so core to Spark that any bug in it would be extremely difficult to debug (yes we have founds bugs in Netty and helped fix those in the past). So unless there are significant benefits, please do NOT update to 4.1.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16888: [WIP] [SPARK-19552] [BUILD] Upgrade Netty version to 4.1...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the issue:
https://github.com/apache/spark/pull/16888
@a-roberts Spark doesn't use Netty's SSL handler, so it won't be a problem.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16888: [SPARK-19552] [BUILD] Upgrade Netty version to 4.1.8 fin...
Posted by mridulm <gi...@git.apache.org>.
Github user mridulm commented on the issue:
https://github.com/apache/spark/pull/16888
Jenkins, retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16888: [WIP] [SPARK-19552] [BUILD] Upgrade Netty version to 4.1...
Posted by drcrallen <gi...@git.apache.org>.
Github user drcrallen commented on the issue:
https://github.com/apache/spark/pull/16888
taken over by https://github.com/apache/spark/pull/19884 ?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org