HADOOP-16986. S3A to not need wildfly on the classpath. #1948
Conversation
|
Sorry, I hadn't realised the previous patch was going to require wildfly I intend to get this patch into branch 3.3 as well, so only 3.3.0 has the Compared to #1929, this patch doesn't move wildfly dependency up from runtime to compile -this should not be needed. I've verified it gets through the tools-list and hadoop-cloud-storage dependencies, and am building a full release to see what goes on there. then I'll do the azure and s3a test runs. |
|
I don't agree with this approach.
Why? The whole point of openssl mode is that if you enable it, you want the code to fail if openssl cannot be setup. The point of default mode is that it should be best effort.
Not sure I understand the concern with the classpath issues. Sure, wildfly is another dependency, but it has no transitive dependencies, so not sure what the harm is with including it. It doesn't do any weird shading of other dependencies either. This also forces users to pick which version of wildfly to use, rather than S3A saying we support version 'x.y.z'. |
|
hmm I guess the only third-party dependency that hadoop-aws has is the aws sdk, so I can see the hesitation in adding another dependency. on the other hand, the wildfly jar is just a runtime dependency, not compile time. the main concern I have is with the behavior change to the |
There was a problem hiding this comment.
Wouldn't it be better to move the code within try-catch into a new method so that the same can be resued for the case openSSL as well.
There was a problem hiding this comment.
I'd put it here so that if someone asked for openSSL and there were problems, it really would fail. It was in default where wildfly load (and pointer errors) triggered catch and downgrade.
For the S3A Binding there already was some classpath catching, so I Added it there too so that any failure here just falls back to the default AWS SDK binding. For abfs I've left it alone
1 .It's another dependency that everyone downstream has to get right issues 3 & 4 is critical for me, even on the "this is harmless" execution path, wildfly on the CP could break things. Yes, that's probably a bug in wildfly, and it's fixed now. but that doesn't mean that a new openSSL release will cause it to come back.
bq. the main concern I have is with the behavior change to the openssl option.) An ASF Hadoop release with wildfly support hasn't shipped. There is no behaviour change. Yes, there may be changes with releases by other entities, but as past changes have long determined, that cannot be a reason to hole back changes in the hadoop codebase. bq. I can see the hesitation in adding another dependency. on the other hand, the wildfly jar is just a runtime dependency, not compile time. If you need it on the classpath downstream the difference between compile and runtime is moot. If it is not there, your code doesn't work. If it is there and incompatible with your next distribution, your application breaks. This is not a transparent change. Put differently: If I had noticed the previous patch mandated widfly as a dependency, it would have been held back for an update to remove that requirement.
The S3A Binding already handled lots of failures including class not found. I'm just expanding it a bit, with the goal of resilience. |
|
Test results: all good, the usual S3A versioned bucket test failure for which I've a patch outstanding; abfs happy and fast (Trivia: maven test launch doesn't work with AWS corretto JDK8 distro) |
|
+email from Sahil says that it only gets through the HADOOP_OPTIONAL_TOOLS env var if its listed as compile time; somehow the bash stitching together needs it. So @bilaharith's change does need to come in here too |
steveloughran
changed the title
|
LGTM, should we mention this somewhere in the docs? maybe troubleshooting? |
* completely remove wildfly jar as a dependency of hadoop-aws * socket factory to catch and swallow all * and verify that in tests * tests which also verify that in the default factory mode, classloader errors are caught and downgrade to jvm * and in openssl mode, the s3a binding explicitly catches these failures and handles too This means s3a: when widlfy doesn't load for classpath or wildfly/openssl issues, S3A will downgrade to jvm, irrespective of SSL mode option. abfs: when widlfy doesn't load for classpath or wildfly/openssl issues, default: swallow and downgrade to jvm openssl: exception thrown; you can't talk to azure. Yes, these are different behaviours. But abfs has always mandated the wildfly library S3A has never done this and I do not want to start now. If people/project downstream want to use it -fine. If they are running on the machines where open SSL is found and is compatible -they will actually see a speed up. Without that, it is utterly superfluous and simply any other source of class path dependency and stack trace issues. Sorry, I hadn't realised the previous patch was going to require wildfly everywhere; I wouldn't make sure that was an otherwise. I intend to get this patch into branch 3.3 as well, so only 3.3.0 has the extra dependency. Change-Id: I74c4b8a1876b5c28d7d96fe3a6dc8e22ffbdc1f7
steveloughran
force-pushed
the
s3/HADOOP-16855-wildfly-jar
branch
from
55a2a77 to
4a0307e
Compare
steveloughran
changed the title
|
hmm. docs. yeah. you're probably right. renamed this patch to a new JIRA as I've merged @bilaharith's original patch in to branch-3.3+ |
openssl will not downgrade in AWS client setup * cover in docs, including troubleshooting * cleanup of duplicate code in DelegatingSSLSocketFactory * tests cover failure modes * add a way to reset the static DelegatingSSLSocketFactory so that the tests can actually force a reload. Change-Id: I25d09d42cd87fe7e9909cff1e80141bfea69724a
|
The latest patch does say "if you ask for openssl, enjoy the stack trace"; docs explain why you get the stack and actions to take. |
|
💔 -1 overall
This message was automatically generated. |
hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/performance.md
Outdated
Show resolved
Hide resolved
|
tested: s3 london with |
fix checkstyle and site Change-Id: I06a7fca5ec38aacdd53cbc9edd6c7794ca73adba
not done a test rerun except the new test suite from the IDE, one where I haven't yet done a mvn import to pick up the changes in classpaths of s3a. Notable that on the mac openssl does seem to be found in this situation. |
|
🎊 +1 overall
This message was automatically generated. |
|
I tested the patch on CentOS 8 and openssl-1.1.1-8.el8.x86_64. I got expected behaviour. Without wildfly, With wildfly-openssl-1.0.10.Final in the classpath, it is used by supplying |
|
LGTM. TestOpenSSLClasspaths passed. I got no relevant failure on Should this be in 3.3.0? |
|
@iwasakims -nice to see tokyo tested; been playing with stockholm today after some reports of trouble with our SDK version, which predates the region.
I'm thinking: yes, otherwise we will add a new downstream dependency. Which should go into the release notes. I've been playing with this a little bit more, based on Sahil's comments about env vars. Once you take wildfly off the compile CP of hadoop-aws, it isn't picked up on the command line in ~/.hadoop-env and the "hadoop s3guard" command doesn't find it either so unless you get wildfly into your CP in some other way, you can't use the pure openssl binding from the CLI. "default" is fine, just not "openssl" What to do? I like the resilience we've added here, but worry about whether its now overreacting Maybe we should
|
but we are resilient to its absence. Change-Id: I369489ce11f18624e13bc3e4319a7bd4d8ce1389
|
last test run (-s3guard, auth scale) with s3 ireland -proxy config tests filed (JIRA issued), and we got a failure of a job commit in an MR job. Got more logs there and hope to make progress next week HADOOP-16798 |
|
💔 -1 overall
This message was automatically generated. |
|
Thanks! |
steveloughran
changed the title
Contributed by Steve Loughran. This is a successor to HADOOP-16346, which enabled the S3A connector to load the native openssl SSL libraries for better HTTPS performance. That patch required wildfly.jar to be on the classpath. This update: * Makes wildfly.jar optional except in the special case that "fs.s3a.ssl.channel.mode" is set to "openssl" * Retains the declaration of wildfly.jar as a compile-time dependency in the hadoop-aws POM. This means that unless explicitly excluded, applications importing that published maven artifact will, transitively, add the specified wildfly JAR into their classpath for compilation/testing/ distribution. This is done for packaging and to offer that optional speedup. It is not mandatory: applications importing the hadoop-aws POM can exclude it if they choose. Change-Id: I7ed3e5948d1e10ce21276b3508871709347e113d
Contributed by Steve Loughran. This is a successor to HADOOP-16346, which enabled the S3A connector to load the native openssl SSL libraries for better HTTPS performance. That patch required wildfly.jar to be on the classpath. This update: * Makes wildfly.jar optional except in the special case that "fs.s3a.ssl.channel.mode" is set to "openssl" * Retains the declaration of wildfly.jar as a compile-time dependency in the hadoop-aws POM. This means that unless explicitly excluded, applications importing that published maven artifact will, transitively, add the specified wildfly JAR into their classpath for compilation/testing/ distribution. This is done for packaging and to offer that optional speedup. It is not mandatory: applications importing the hadoop-aws POM can exclude it if they choose. Change-Id: I7ed3e5948d1e10ce21276b3508871709347e113d
…pache#1948) Contributed by Steve Loughran This is a successor to HADOOP-16346, which enabled the S3A connector to load the native openssl SSL libraries for better HTTPS performance. That patch required wildfly.jar to be on the classpath. This update: * Makes wildfly.jar optional except in the special case that "fs.s3a.ssl.channel.mode" is set to "openssl" * Retains the declaration of wildfly.jar as a compile-time dependency in the hadoop-aws POM. This means that unless explicitly excluded, applications importing that published maven artifact will, transitively, add the specified wildfly JAR into their classpath for compilation/testing/ distribution. This is done for packaging and to offer that optional speedup. It is not mandatory: applications importing the hadoop-aws POM can exclude it if they choose. Change-Id: I76e7c2baa209b4c826b96532243044de16470375
6 participants
Contributed by Steve Loughran
Successor to HADOOP-16346.
classloader errors are caught and downgrade to jvm
these failures and handles too
This means
s3a:
when widlfy doesn't load for classpath or wildfly/openssl issues, S3A will
downgrade to jvm, irrespective of SSL mode option.
abfs:
when widlfy doesn't load for classpath or wildfly/openssl issues,
default: swallow and downgrade to jvm
openssl: exception thrown; you can't talk to azure.
Yes, these are different behaviours. But abfs has always mandated the wildfly
library S3A has never done this and starting now will only surprise people. If
people/project downstream want to use it -fine. If they are running on the
machines where open SSL is found and is compatible -they will actually see a
speed up. Without that, it is utterly superfluous and simply any other source
of class path dependency and stack trace issues.
Change-Id: I74c4b8a1876b5c28d7d96fe3a6dc8e22ffbdc1f7