HDFS-16453. Upgrade okhttp from 2.7.5 to 4.9.3 #4229

ashutoshcipher · 2022-04-23T23:23:31Z

Raising PR to check okhttp upgrade possibility to 4.9.3

steveloughran · 2022-04-24T17:03:17Z

the spotbugs failures are probably less a regression the lib as improved nullable declarations in the API letting spotbugs know the code is bad. that bit of okta api use needs to handle null responses better. probably it should check the return type is text/json, as some oauth endpoints (hello microsoft live) will return 200 and human-friendly html on auth problems.

aajisaka

The change updates the dependency, so you need to update LICENSE-binary as well.

...rc/main/java/org/apache/hadoop/hdfs/web/oauth2/ConfRefreshTokenBasedAccessTokenProvider.java

hadoop-project/pom.xml

...rc/main/java/org/apache/hadoop/hdfs/web/oauth2/ConfRefreshTokenBasedAccessTokenProvider.java

…-resources for OkHttpClient

steveloughran · 2022-04-26T18:50:04Z

commented.

checkstyle needs to be happy, along with javac.
spotbugs still thinks there is a problem. what is it that it is warning about?

i'm worried about adding kotlin everywhere. looking at the mvnrepo declarations it is (a) not optional and (b) about 1.5MB including transitive dependencies. so nothing much. my main concern is what pain does it cause downstream. we've had to tag this as an incompatible change just to add in the release notes about where it is used/needed

ashutoshcipher · 2022-04-26T20:48:55Z

commented.

checkstyle needs to be happy, along with javac.

spotbugs still thinks there is a problem. what is it that it is warning about?

i'm worried about adding kotlin everywhere. looking at the mvnrepo declarations it is (a) not optional and (b) about 1.5MB including transitive dependencies. so nothing much. my main concern is what pain does it cause downstream. we've had to tag this as an incompatible change just to add in the release notes about where it is used/needed

Thanks for your comment @steveloughran

Checkstyle would be happy with my last commit.
Javac -

hadoop-hdfs-project/hadoop-hdfs client/src/main/java/org/apache/hadoop/hdfs/web/oauth2/CredentialBasedAccessTokenProvider.java:109:36:[deprecation] create(MediaType,String) in RequestBody has been deprecated - Handled this in latest commit.
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/RequestHedgingProxyProvider.java:229:76:[unchecked] unchecked cast -I believe this is not due this change, can see the same cast warning in my local on trunk as well. May be we can create a separate JIRA to handle this.

spotbugs still thinks there is a problem. what is it that it is warning about?
I already used try-with-resources for OkHttpClient and added request checks as well. The warning is about NPE case.
I agree with your view on Kotlin.

LICENSE-binary

aajisaka · 2022-05-02T17:51:30Z

spotbugs still thinks there is a problem. what is it that it is warning about?
I already used try-with-resources for OkHttpClient and added request checks as well. The warning is about NPE case.

Reading the source code, responseBody.body() is always non-null. However, spotbugs thinks responseBody.body() may be null and it warns "need null check". I think it is because Spotbugs failed to analyze the okhttp classes from Kotlin.

I think we can simply ignore the warnings. @ashutoshcipher would you add some entries to ignore the spotbugs warning in hadoop-hdfs-project/hadoop-hdfs-client/dev-support/findbugsExcludeFile.xml?

...rc/main/java/org/apache/hadoop/hdfs/web/oauth2/ConfRefreshTokenBasedAccessTokenProvider.java

…conventions

ashutoshcipher · 2022-05-04T12:40:07Z

spotbugs still thinks there is a problem. what is it that it is warning about?
I already used try-with-resources for OkHttpClient and added request checks as well. The warning is about NPE case.

Reading the source code, responseBody.body() is always non-null. However, spotbugs thinks responseBody.body() may be null and it warns "need null check". I think it is because Spotbugs failed to analyze the okhttp classes from Kotlin.

I think we can simply ignore the warnings. @ashutoshcipher would you add some entries to ignore the spotbugs warning in hadoop-hdfs-project/hadoop-hdfs-client/dev-support/findbugsExcludeFile.xml?

Done

aajisaka · 2022-05-07T22:01:43Z

@ashutoshcipher Thank you for your update. Would you fix the error in https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4229/10/artifact/out/patch-mvninstall-root.txt ?

The error message starts with

[INFO] --- exec-maven-plugin:1.3.1:exec (check-jar-contents) @ hadoop-client-check-invariants ---
[ERROR] Found artifact with unexpected contents: '/home/jenkins/jenkins-home/workspace/hadoop-multibranch_PR-4229/ubuntu-focal/src/hadoop-client-modules/hadoop-client-runtime/target/hadoop-client-runtime-3.4.0-SNAPSHOT.jar'
    Please check the following and either correct the build or update
    the allowed list with reasoning.

    okhttp3/
    okhttp3/Request$Builder.class
    okhttp3/Dispatcher.class
    okhttp3/Headers$Companion.class

aajisaka · 2022-05-10T08:50:16Z

Re-run the precommit job: https://ci-hadoop.apache.org/blue/organizations/jenkins/hadoop-multibranch/detail/PR-4229/12/pipeline

aajisaka · 2022-05-10T09:52:51Z

The error is

/home/jenkins/jenkins-agent/workspace/hadoop-multibranch_PR-4229: Read-only file system

I made the hadoop10 node offline https://ci-hadoop.apache.org/computer/hadoop10/

aajisaka · 2022-05-10T09:53:29Z

Re-run again: https://ci-hadoop.apache.org/blue/organizations/jenkins/hadoop-multibranch/detail/PR-4229/13/pipeline

ashutoshcipher · 2022-05-14T01:08:21Z

@aajisaka - Thanks for re-run. It seems to be completed. Can you please review. Thanks.

aajisaka · 2022-05-17T16:53:11Z

The test failures looks related to HADOOP-18222, but I want to run the precommit job again to validate.
https://ci-hadoop.apache.org/blue/organizations/jenkins/hadoop-multibranch/detail/PR-4229/14/pipeline/

hadoop-yetus · 2022-05-18T15:57:31Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	46m 20s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 1s		No case conflicting files found.
+0 🆗	codespell	0m 1s		codespell was not available.
+0 🆗	shellcheck	0m 1s		Shellcheck was not available.
+0 🆗	shelldocs	0m 1s		Shelldocs was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
-1 ❌	test4tests	0m 0s		The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
			_ trunk Compile Tests _
+0 🆗	mvndep	15m 59s		Maven dependency ordering for branch
+1 💚	mvninstall	28m 25s		trunk passed
+1 💚	compile	21m 47s		trunk passed
+1 💚	checkstyle	4m 30s		trunk passed
+1 💚	mvnsite	20m 22s		trunk passed
+1 💚	javadoc	8m 49s		trunk passed
+0 🆗	spotbugs	0m 27s		branch/hadoop-project no spotbugs output file (spotbugsXml.xml)
+0 🆗	spotbugs	0m 27s		branch/hadoop-client-modules/hadoop-client no spotbugs output file (spotbugsXml.xml)
+1 💚	shadedclient	58m 15s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 40s		Maven dependency ordering for patch
+1 💚	mvninstall	26m 51s		the patch passed
+1 💚	compile	21m 6s		the patch passed
+1 💚	javac	21m 6s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	4m 26s		the patch passed
+1 💚	mvnsite	20m 0s		the patch passed
+1 💚	xml	0m 7s		The patch has no ill-formed XML file.
+1 💚	javadoc	8m 30s		the patch passed
+0 🆗	spotbugs	0m 26s		hadoop-project has no data from spotbugs
+0 🆗	spotbugs	0m 27s		hadoop-client-modules/hadoop-client has no data from spotbugs
+1 💚	shadedclient	58m 29s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
-1 ❌	unit	1030m 58s	/patch-unit-root.txt	root in the patch passed.
+1 💚	asflicense	2m 19s		The patch does not generate ASF License warnings.
		1391m 35s

Reason	Tests
Failed junit tests	hadoop.mapred.TestLocalDistributedCacheManager
	hadoop.service.launcher.TestServiceInterruptHandling
	hadoop.crypto.TestCryptoCodec
	hadoop.crypto.TestCryptoStreamsWithOpensslSm4CtrCryptoCodec

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4229/14/artifact/out/Dockerfile
GITHUB PR	#4229
Optional Tests	dupname asflicense codespell shellcheck shelldocs compile javac javadoc mvninstall mvnsite unit shadedclient xml spotbugs checkstyle
uname	Linux a5b4eac27144 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 / `3de6058`
Default Java	Red Hat, Inc.-1.8.0_332-b09
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4229/14/testReport/
Max. process+thread count	2272 (vs. ulimit of 5500)
modules	C: hadoop-project hadoop-hdfs-project/hadoop-hdfs-client hadoop-client-modules/hadoop-client hadoop-tools/hadoop-azure-datalake . U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4229/14/console
versions	git=2.9.5 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.

aajisaka · 2022-05-19T04:19:03Z

Failed tests are not related to the patch.

TestLocalDistributedCacheManager: Failing on trunk. Filed MAPREDUCE-7383
TestServiceInterruptHandling, TestCryptoCodec, TestCryptoStreamsWithOpensslSm4CtrCryptoCodec: Passed on my local.

aajisaka · 2022-05-19T04:28:47Z

@ashutoshcipher Would you remove this line? I'm +1 if that is addressed.

hadoop/hadoop-tools/hadoop-azure-datalake/pom.xml

Line 32 in 516f36c

<okHttpVersion>${okhttp.version}</okHttpVersion>

ashutoshcipher · 2022-05-19T14:51:34Z

@ashutoshcipher Would you remove this line? I'm +1 if that is addressed.

hadoop/hadoop-tools/hadoop-azure-datalake/pom.xml

Line 32 in 516f36c

<okHttpVersion>${okhttp.version}</okHttpVersion>

Thanks for pointing it out, I have addressed it.

hadoop-yetus · 2022-05-20T14:35:04Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 56s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	shelldocs	0m 0s		Shelldocs was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
-1 ❌	test4tests	0m 0s		The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
			_ trunk Compile Tests _
+0 🆗	mvndep	14m 55s		Maven dependency ordering for branch
+1 💚	mvninstall	28m 3s		trunk passed
+1 💚	compile	25m 1s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	compile	21m 32s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	4m 31s		trunk passed
+1 💚	mvnsite	19m 59s		trunk passed
+1 💚	javadoc	8m 27s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	7m 31s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+0 🆗	spotbugs	0m 27s		branch/hadoop-project no spotbugs output file (spotbugsXml.xml)
+0 🆗	spotbugs	0m 27s		branch/hadoop-client-modules/hadoop-client no spotbugs output file (spotbugsXml.xml)
+1 💚	shadedclient	57m 21s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 37s		Maven dependency ordering for patch
+1 💚	mvninstall	26m 23s		the patch passed
+1 💚	compile	24m 18s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javac	24m 18s		the patch passed
+1 💚	compile	21m 34s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	21m 34s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	4m 23s		the patch passed
+1 💚	mvnsite	19m 34s		the patch passed
+1 💚	shellcheck	0m 0s		No new issues.
+1 💚	xml	0m 6s		The patch has no ill-formed XML file.
+1 💚	javadoc	8m 21s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	7m 25s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+0 🆗	spotbugs	0m 26s		hadoop-project has no data from spotbugs
+0 🆗	spotbugs	0m 26s		hadoop-client-modules/hadoop-client has no data from spotbugs
+1 💚	shadedclient	57m 47s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
-1 ❌	unit	1058m 18s	/patch-unit-root.txt	root in the patch passed.
+1 💚	asflicense	2m 18s		The patch does not generate ASF License warnings.
		1433m 47s

Reason	Tests
Failed junit tests	hadoop.mapred.TestLocalDistributedCacheManager

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4229/15/artifact/out/Dockerfile
GITHUB PR	#4229
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell xml spotbugs checkstyle shellcheck shelldocs
uname	Linux da7c2ef22ff2 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 / `a7c89ff`
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-4229/15/testReport/
Max. process+thread count	2248 (vs. ulimit of 5500)
modules	C: hadoop-project hadoop-hdfs-project/hadoop-hdfs-client hadoop-client-modules/hadoop-client hadoop-tools/hadoop-azure-datalake . U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4229/15/console
versions	git=2.25.1 maven=3.6.3 shellcheck=0.7.0 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

aajisaka · 2022-05-20T17:53:46Z

Merged. Thank you @ashutoshcipher for your contribution and thanks @steveloughran for your review.

Co-authored-by: Ashutosh Gupta <ashugpt@amazon.com> Signed-off-by: Akira Ajisaka <aajisaka@apache.org> (cherry picked from commit fb910bd) Conflicts: hadoop-project/pom.xml

pan3793 · 2022-06-30T06:19:46Z

okhttp 3.14.9 is the latest version which does not depend on kotlin

pan3793 · 2022-07-11T04:48:36Z

The okhttp is used in 1 place but involves a language runtime deps, do we really need this upgrading? And since Hadoop already has org.apache.httpcomponents:httpclient, do we need okhttp?

Co-authored-by: Ashutosh Gupta <ashugpt@amazon.com> Signed-off-by: Akira Ajisaka <aajisaka@apache.org>

steveloughran · 2023-02-23T19:23:21Z

was this the PR which added kotlin as a dependency?

if so: please remember to validate license then update the LICENSE-binary file once you have verified it is compatible.

HADOOP-18069. okhttp@2.7.5 to 4.9.3

ca1de27