Skip to content

HBASE-23015 : Moving from Jackson2 to shaded Gson (Backport HBASE-20587) - #616

Merged
busbey merged 7 commits into
apache:branch-1from
virajjasani:HBASE-23015-branch-1
Sep 26, 2019
Merged

HBASE-23015 : Moving from Jackson2 to shaded Gson (Backport HBASE-20587)#616
busbey merged 7 commits into
apache:branch-1from
virajjasani:HBASE-23015-branch-1

Conversation

@virajjasani

Copy link
Copy Markdown
Contributor

No description provided.

@Apache-HBase

This comment has been minimized.

Comment threadhbase-common/pom.xml Outdated
Comment threadhbase-shaded/hbase-shaded-server/pom.xml Outdated
Comment threadhbase-shaded/hbase-shaded-testing-util-tester/pom.xml Outdated
Comment threadhbase-shaded/hbase-shaded-testing-util/pom.xml Outdated
Comment threadhbase-testing-util/pom.xml Outdated
@busbey

Copy link
Copy Markdown
Contributor

Is this a backport of HBASE-20587 or all new?

@virajjasanivirajjasani changed the title HBASE-23015 : Moving from Jackson2 to shaded GsonHBASE-23015 : Moving from Jackson2 to shaded Gson (Backport HBASE-20587)Sep 12, 2019
@virajjasani

Copy link
Copy Markdown
ContributorAuthor

Except for supplemental-models.xml and adding hbase-shaded-miscellaneous dependency, mostly this is backport of HBASE-20587 for relevant files(a couple of files on branch-1 don't have Jackson2 and hence, unchanged in this PR)

Comment threadhbase-testing-util/pom.xml
@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@busbey

Copy link
Copy Markdown
Contributor

This is looking great! I still need to try using it with a downstream project and after that I think it'll be good to go.

When we commit this it needs to go to all branches-1.

@virajjasani

virajjasani commented Sep 14, 2019

Copy link
Copy Markdown
ContributorAuthor

This is looking great! I still need to try using it with a downstream project and after that I think it'll be good to go.

When we commit this it needs to go to all branches-1.

Sure, in the meanwhile let me work on preparing backport patch for branch-1.4 and 1.3 if they have significant changes involved?

Update: It seems backport to branch-1.4 doesn't require extra effort, however branch-1.3 does

@busbey

Copy link
Copy Markdown
Contributor

ugh. Can't believe I didn't think of jdk requirements. Just built this on branch-1 with -P release so I'd have shaded artifacts to test downstream

[INFO] --- maven-enforcer-plugin:3.0.0-M1:enforce (hadoop-profile-min-maven-min-java-banned-xerces) @ hbase-common ---
[INFO] Restricted to JDK 1.7 yet org.apache.hbase.thirdparty:hbase-shaded-miscellaneous:jar:2.2.1:compile contains org/apache/hbase/thirdparty/com/google/common/annotations/Beta.class targeted to JDK 1.8
[WARNING] Rule 4: org.apache.maven.plugins.enforcer.EnforceBytecodeVersion failed with message:
HBase has unsupported dependencies.
HBase requires that all dependencies be compiled with version 1.7 or earlier
of the JDK to properly build from source. You appear to be using a newer dependency. You can use
either "mvn -version" or "mvn enforcer:display-info" to verify what version is active.
Non-release builds can temporarily build with a newer JDK version by setting the
'compileSource' property (eg. mvn -DcompileSource=1.8 clean package).
Found Banned Dependency: org.apache.hbase.thirdparty:hbase-shaded-miscellaneous:jar:2.2.1
Use 'mvn dependency:tree' to locate the source of the banned dependencies.

Let me think about what's going to be faster here. We could exclude the stuff other than gson or we could see if updating the dependencies of hbase-thirdparty to be jdk7 compatible. I know guava still works for jdk7 if we use the "android" variant instead of "jre".

@busbey

Copy link
Copy Markdown
Contributor

I have some ideas on moving this forward. discussion on HBASE-23052

@Apache-HBase

This comment has been minimized.

@busbey

Copy link
Copy Markdown
Contributor

Okay! a new version of the hbase-thirdparty library is out now and includes a jdk7 compatible relocated GSON.

I just published it on the ASF nexus, so it might take a bit for it to show up in maven central. When it does you should be able to update this PR to use the following dependency:

<dependency>
<groupId>org.apache.hbase.thirdparty</groupId>
<artifactId>hbase-shaded-gson</artifactId>
<version>3.0.0</version>
</dependency>

@virajjasani

Copy link
Copy Markdown
ContributorAuthor

That's great! Thanks @busbey
Sure I will be on it, soon as my local build passes, will commit it

@virajjasani

Copy link
Copy Markdown
ContributorAuthor

Comment threadhbase-testing-util/pom.xml
@busbey

Copy link
Copy Markdown
Contributor

Building with -P release fails while checking the shaded jars for correctness.

Need to update hbase-shaded/hbase-shaded-check-invariants/src/test/resources/ensure-jars-have-correct-contents.sh to allow for packages in org/apache/hbase since that's what the hbase-thirdparty stuff uses (as compared to org/apache/hadoop/hbase for the rest of our stuff). you can either cherry-pick b3f353c or just make the change yourself.

@busbeybusbey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

with the addition of the shaded jar check, I can successfully build with the release profile and verify that the jars we produce are usable downstream (checked by building YCSB with some local changes to allow use of the shaded testing util).

Also the module is visible in Maven Central now:

https://search.maven.org/artifact/org.apache.hbase.thirdparty/hbase-shaded-gson/3.0.0/jar

@busbey

Copy link
Copy Markdown
Contributor

I plan to merge this tonight and get it back-ported to branch-1.4 as well, unless someone else has more feedback.

@Apache-HBase

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
💙reexec1m 28sDocker mode activated.
_ Prechecks _
💚dupname0m 1sNo case conflicting files found.
💙shelldocs0m 0sShelldocs was not available.
💚hbaseanti0m 0sPatch does not have any anti-patterns.
💚@author0m 0sThe patch does not contain any @author tags.
💚test4tests0m 0sThe patch appears to include 4 new or modified test files.
_ branch-1 Compile Tests _
💙mvndep6m 21sMaven dependency ordering for branch
💚mvninstall3m 42sbranch-1 passed
💚compile11m 28sbranch-1 passed with JDK v1.8.0_222
💔compile0m 17sroot in branch-1 failed with JDK v1.7.0_232.
💚checkstyle9m 56sbranch-1 passed
💚shadedjars3m 11sbranch has no errors when building our shaded downstream artifacts.
💚javadoc4m 27sbranch-1 passed with JDK v1.8.0_222
💚javadoc6m 45sbranch-1 passed with JDK v1.7.0_232
💙spotbugs11m 25sUsed deprecated FindBugs config; considering switching to SpotBugs.
💙findbugs0m 22sbranch/hbase-resource-bundle no findbugs output file (findbugsXml.xml)
💙findbugs0m 20sbranch/hbase-testing-util no findbugs output file (findbugsXml.xml)
💙findbugs0m 19sbranch/hbase-shaded/hbase-shaded-server no findbugs output file (findbugsXml.xml)
💙findbugs0m 20sbranch/hbase-shaded/hbase-shaded-testing-util no findbugs output file (findbugsXml.xml)
💙findbugs0m 12sbranch/hbase-shaded/hbase-shaded-testing-util-tester no findbugs output file (findbugsXml.xml)
💙findbugs0m 18sbranch/hbase-shaded/hbase-shaded-check-invariants no findbugs output file (findbugsXml.xml)
_ Patch Compile Tests _
💙mvndep0m 27sMaven dependency ordering for patch
💚mvninstall2m 6sthe patch passed
💚compile11m 6sthe patch passed with JDK v1.8.0_222
💚javac11m 6sthe patch passed
💔compile0m 17sroot in the patch failed with JDK v1.7.0_232.
💔javac0m 17sroot in the patch failed with JDK v1.7.0_232.
💚checkstyle0m 12sThe patch passed checkstyle in hbase-resource-bundle
💚checkstyle0m 30sThe patch passed checkstyle in hbase-common
💚checkstyle0m 37sThe patch passed checkstyle in hbase-client
💚checkstyle1m 48shbase-server: The patch generated 0 new + 256 unchanged - 25 fixed = 256 total (was 281)
💚checkstyle0m 12sThe patch passed checkstyle in hbase-testing-util
💚checkstyle0m 9sThe patch passed checkstyle in hbase-shaded-server
💚checkstyle0m 13sThe patch passed checkstyle in hbase-shaded-testing-util
💚checkstyle0m 12sThe patch passed checkstyle in hbase-shaded-testing-util-tester
💚checkstyle0m 13sThe patch passed checkstyle in hbase-shaded-check-invariants
💚checkstyle6m 1sroot: The patch generated 0 new + 256 unchanged - 25 fixed = 256 total (was 281)
💚shellcheck0m 0sThere were no new shellcheck issues.
💚whitespace0m 0sThe patch has no whitespace issues.
💔xml0m 1sThe patch has 9 ill-formed XML file(s).
💚shadedjars3m 4spatch has no errors when building our shaded downstream artifacts.
💚hadoopcheck5m 12sPatch does not cause any errors with Hadoop 2.8.5 2.9.2.
💚javadoc4m 23sthe patch passed with JDK v1.8.0_222
💚javadoc6m 50sthe patch passed with JDK v1.7.0_232
💙findbugs0m 13shbase-resource-bundle has no data from findbugs
💙findbugs0m 20shbase-testing-util has no data from findbugs
💙findbugs0m 19shbase-shaded/hbase-shaded-server has no data from findbugs
💙findbugs0m 21shbase-shaded/hbase-shaded-testing-util has no data from findbugs
💙findbugs0m 13shbase-shaded/hbase-shaded-testing-util-tester has no data from findbugs
💙findbugs0m 17shbase-shaded/hbase-shaded-check-invariants has no data from findbugs
_ Other Tests _
💔unit162m 49sroot in the patch failed.
💚asflicense4m 27sThe patch does not generate ASF License warnings.
301m 43s
ReasonTests
XMLParsing Error(s):
hbase-client/pom.xml
hbase-common/pom.xml
hbase-resource-bundle/src/main/resources/supplemental-models.xml
hbase-server/pom.xml
hbase-shaded/hbase-shaded-server/pom.xml
hbase-shaded/hbase-shaded-testing-util-tester/pom.xml
hbase-shaded/hbase-shaded-testing-util/pom.xml
hbase-testing-util/pom.xml
pom.xml
Failed junit testshadoop.hbase.replication.TestReplicationSmallTests
SubsystemReport/Notes
DockerClient=19.03.0 Server=19.03.0 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-616/6/artifact/out/Dockerfile
GITHUB PR#616
Optional Testsdupname asflicense javac javadoc unit shadedjars hadoopcheck xml compile spotbugs findbugs hbaseanti checkstyle shellcheck shelldocs
unameLinux a3c1d4838deb 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personality/home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-616/out/precommit/personality/provided.sh
git revisionbranch-1 / 771e184
Default Java1.7.0_232
Multi-JDK versions/usr/lib/jvm/zulu-8-amd64:1.8.0_222 /usr/lib/jvm/zulu-7-amd64:1.7.0_232
compilehttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-616/6/artifact/out/branch-compile-root-jdk1.7.0_232.txt
compilehttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-616/6/artifact/out/patch-compile-root-jdk1.7.0_232.txt
javachttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-616/6/artifact/out/patch-compile-root-jdk1.7.0_232.txt
xmlhttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-616/6/artifact/out/xml.txt
unithttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-616/6/artifact/out/patch-unit-root.txt
Test Resultshttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-616/6/testReport/
Max. process+thread count3924 (vs. ulimit of 10000)
modulesC: hbase-resource-bundle hbase-common hbase-client hbase-server hbase-testing-util hbase-shaded/hbase-shaded-server hbase-shaded/hbase-shaded-testing-util hbase-shaded/hbase-shaded-testing-util-tester hbase-shaded/hbase-shaded-check-invariants . U: .
Console outputhttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-616/6/console
versionsgit=1.9.1 maven=3.0.5 shellcheck=0.7.0 findbugs=3.0.1
Powered byApache Yetus 0.11.0 https://yetus.apache.org

This message was automatically generated.

@busbey

Copy link
Copy Markdown
Contributor

compile | 0m 17s | root in branch-1 failed with JDK v1.7.0_232.

This is concerning, but unrelated. I'll chase it down later.

xml | 0m 1s | The patch has 9 ill-formed XML file(s).

this is HBASE-22732

Failed junit tests | hadoop.hbase.replication.TestReplicationSmallTests

Not related AFAICT.

@busbey
busbey merged commit f77c14d into apache:branch-1Sep 26, 2019
asfgit pushed a commit that referenced this pull request Sep 26, 2019
* Backport HBASE-20587
* moving to shaded gson with jdk7 compatibility
* Include jackson-mapper-asl for testing-util because of Hadoop
* Update shaded jar check to allow hbase-thirdparty libs
Signed-off-by: Sean Busbey <busbey@apache.org>
Co-authored-by: Duo Zhang <zhangduo@apache.org>
asfgit pushed a commit that referenced this pull request Sep 26, 2019
* Backport HBASE-20587
* moving to shaded gson with jdk7 compatibility
* Include jackson-mapper-asl for testing-util because of Hadoop
* Update shaded jar check to allow hbase-thirdparty libs
Signed-off-by: Sean Busbey <busbey@apache.org>
Co-authored-by: Duo Zhang <zhangduo@apache.org>
Sign up for freeto 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

@virajjasani@Apache-HBase@busbey