-
Notifications
You must be signed in to change notification settings - Fork 9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
YARN-11786. Upgrade hadoop-yarn-server-timelineservice-hbase-tests to Support Trunk Compilation and Remove compatible hadoop version. #7453
Open
slfan1989
wants to merge
6
commits into
apache:trunk
Choose a base branch
from
slfan1989:remove-compatible-hadoop-version
base: trunk
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+991
−296
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
c264409
REMOVE-COMPATIBLE-HADOOP-VERSION
slfan1989 54f79ce
fix junit test.
slfan1989 07e1538
fix junit test.
slfan1989 6049652
YARN-11786. Improve Some Code.
slfan1989 75fc637
YARN-11786. Upgrade hadoop-yarn-server-timelineservice-hbase-tests to…
slfan1989 6c24c0c
Merge branch 'apache:trunk' into remove-compatible-hadoop-version
slfan1989 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar with differences in the new Jersey version, but it seems like the prior code was much more driven by introspection and annotations instead of needing to register a lot of custom readers. Is there any way to make it more like the prior code? I'm concerned about risk of backward-incompatible changes in the manual approach and potential error-prone evolution of the code if fields are changed in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good question. I believe we need to update some parts of the code to align with Jersey 2's standards, as certain parts of the code are not well-supported by the latest standards.
I first became aware of the issue with Jersey during the release of Hadoop 3.4.0, when aajisaka mentioned that, with Hadoop supporting JDK 11, there was an area that needed optimization — Jersey support. For more details, you can refer to HADOOP-15984 (updating Jersey from 1.19 to 2.x).
Jersey is a framework that supports RESTful WebService. When upgrading from version 1.x to 2.x, compatibility issues arise because these two versions are completely incompatible, with even basic methods being different.
To address this, we define custom readers because Jersey 2.x only supports automatic JSON conversion for basic data types and does not support automatic conversion for more complex types. More details can be found in the official documentation.
While writing these readers, I used
com.fasterxml.jackson.databind.ObjectMapper
to implement automatic conversion between objects and JSON. However, ObjectMapper cannot handle null pointer exceptions, which leads to the first question — the need to assign initial values to certain variables.After upgrading to JUnit 5, we will configure a new JDK 17 CI pipeline. Following that, I will begin the deeper adaptation and upgrade of Jersey 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slfan1989 , thank you for the reply. This is helpful information.
It sounds like some of this is just unavoidable with this Jersey upgrade. The biggest thing I'm concerned about is the potential to break YARN's user-facing APIs. Please correct me if I'm wrong, but I think we want to maintain full API compatibility in the 3.5.0 release. If so, how can we be confident about this? Maybe we need to spin up some kind of test of a 3.4 client against a 3.5 server?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cnauroth Thank you for your message. We need to ensure the compatibility of the interfaces. For the YARN REST API, most interfaces have no compatibility issues. Even after upgrading to Jersey 2, the changes to the server's WebService are minimal, usually just adding readers and writers. While there are still a few interfaces that need re-validation, I have a general understanding of the areas that need to be checked. We can work together to complete this task, which must be done before the release of 3.5.0.