Skip to content

HADOOP-18245. Extend KMS related exceptions that get mapped to ConnectException#4329

Merged
jojochuang merged 1 commit intoapache:trunkfrom
kerneltime:HADOOP-18245
May 19, 2022
Merged

HADOOP-18245. Extend KMS related exceptions that get mapped to ConnectException#4329
jojochuang merged 1 commit intoapache:trunkfrom
kerneltime:HADOOP-18245

Conversation

@kerneltime
Copy link
Contributor

@kerneltime kerneltime commented May 18, 2022

Description of PR

Based on production workload, we found that it is not enough to map just SSLHandshakeException to ConnectException in Loadbalancing KMS Client but that needs to be extended to SSLExceptions and SocketExceptions.

How was this patch tested?

Updated existing unit tests.

For code changes:

  • Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
  • Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE, LICENSE-binary, NOTICE-binary files?

Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

For future reference the stacktrace was:

22/04/13 16:25:55 WARN kms.LoadBalancingKMSClientProvider: KMS provider at [https://xxxx:16001/kms/v1/] threw an IOException:
javax.net.ssl.SSLException: readHandshakeRecord
at sun.security.ssl.SSLSocketImpl.readHandshakeRecord(SSLSocketImpl.java:1210)

    at sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:373)
    at sun.net.www.protocol.https.HttpsClient.afterConnect(HttpsClient.java:587)
    at sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.connect(AbstractDelegateHttpsURLConnection.java:197)
    at sun.net.www.protocol.https.HttpsURLConnectionImpl.connect(HttpsURLConnectionImpl.java:167)

Caused by: java.net.SocketException: Broken pipe (Write failed)
at java.net.SocketOutputStream.socketWrite0(Native Method)

    at sun.security.ssl.SSLSocketOutputRecord.flush(SSLSocketOutputRecord.java:268)
    at sun.security.ssl.HandshakeOutStream.flush(HandshakeOutStream.java:89)
    at sun.security.ssl.Finished$T12FinishedProducer.onProduceFinished(Finished.java:399)
    at sun.security.ssl.Finished$T12FinishedProducer.produce(Finished.java:374)
    at sun.security.ssl.SSLHandshake.produce(SSLHandshake.java:420)
    at sun.security.ssl.ServerHelloDone$ServerHelloDoneConsumer.consume(ServerHelloDone.java:182) <=======
    at sun.security.ssl.SSLHandshake.consume(SSLHandshake.java:376)

    at sun.security.ssl.SSLSocketImpl.decode(SSLSocketImpl.java:1290)
    at sun.security.ssl.SSLSocketImpl.readHandshakeRecord(SSLSocketImpl.java:1199)
    ... 58 more

@hadoop-yetus
Copy link

🎊 +1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 55s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 codespell 0m 0s codespell was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
+1 💚 test4tests 0m 0s The patch appears to include 1 new or modified test files.
_ trunk Compile Tests _
+1 💚 mvninstall 41m 34s trunk passed
+1 💚 compile 27m 11s trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚 compile 23m 20s trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚 checkstyle 1m 31s trunk passed
+1 💚 mvnsite 2m 3s trunk passed
+1 💚 javadoc 1m 31s trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚 javadoc 1m 7s trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚 spotbugs 3m 5s trunk passed
+1 💚 shadedclient 26m 38s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+1 💚 mvninstall 1m 5s the patch passed
+1 💚 compile 25m 47s the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚 javac 25m 47s the patch passed
+1 💚 compile 22m 47s the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚 javac 22m 47s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
-0 ⚠️ checkstyle 1m 26s /results-checkstyle-hadoop-common-project_hadoop-common.txt hadoop-common-project/hadoop-common: The patch generated 1 new + 9 unchanged - 0 fixed = 10 total (was 9)
+1 💚 mvnsite 2m 0s the patch passed
+1 💚 javadoc 1m 20s the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚 javadoc 1m 4s the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚 spotbugs 3m 2s the patch passed
+1 💚 shadedclient 26m 13s patch has no errors when building and testing our client artifacts.
_ Other Tests _
+1 💚 unit 18m 5s hadoop-common in the patch passed.
+1 💚 asflicense 1m 16s The patch does not generate ASF License warnings.
233m 33s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4329/1/artifact/out/Dockerfile
GITHUB PR #4329
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell
uname Linux 94d3a4056458 4.15.0-175-generic #184-Ubuntu SMP Thu Mar 24 17:48:36 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / 15a97f0
Default Java Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4329/1/testReport/
Max. process+thread count 1252 (vs. ulimit of 5500)
modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4329/1/console
versions git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

@sodonnel sodonnel changed the title HADOOP-18245 Extend KMS related exceptions that get mapped to Connect… HADOOP-18245. Extend KMS related exceptions that get mapped to ConnectException May 19, 2022
// with the KMS server, creating a ConnectException from it,
// so that the FailoverOnNetworkExceptionRetry policy will retry
if (ioe instanceof SSLHandshakeException) {
if (ioe instanceof SSLException || ioe instanceof SocketException) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding more comments for future reference.

@jojochuang jojochuang merged commit 78008bc into apache:trunk May 19, 2022
HarshitGupta11 pushed a commit to HarshitGupta11/hadoop that referenced this pull request Nov 28, 2022
jojochuang pushed a commit to jojochuang/hadoop that referenced this pull request May 23, 2023
…d to ConnectException (apache#4329)

Change-Id: I1608dec7f5003cf89af5e5f7f7962a20a069c050
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments