YARN-11874. ResourceManager REST API backward compatibility with Jersey1 #8033
+8,660
−17,623
Conversation
This file contains hidden or 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
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
@K0K0V0K Thank you for bringing this issue to our attention. I will assist in completing the upgrade for YARN-11874. |
|
💔 -1 overall
This message was automatically generated. |
|
Since Yetus cannot handle very large PRs, we need to temporarily replace it with Ayush’s branch during development. Once this PR is merged, we should revert this change. |
|
💔 -1 overall
This message was automatically generated. |
|
Hi @slfan1989 ! Huge thanks for the help! Some update:
I tried to make jettison working but i was not able to find the right configs to reproduce the Jersey1 API behaviour. |
@K0K0V0K Thank you for this PR! Let’s work together to find a solution to this issue. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
K0K0V0K
changed the title
|
Hi @slfan1989! I think i reached a reviewable phase with this PR. |
Thanks a lot for your great work on this! This PR is really important, and I’ll make it my top priority to follow up. |
There was a problem hiding this comment.
Thank you @K0K0V0K for raising this PR. This really help a lot!
It looks good to me but I do have some clean up comments and some follow up questions.
...r/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/dao/NodeLabelsInfo.java
Show resolved
Hide resolved
...r/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/dao/NodeLabelsInfo.java
Show resolved
Hide resolved
.../apache/hadoop/yarn/server/resourcemanager/federation/TestFederationRMStateStoreService.java
Outdated
Show resolved
Hide resolved
...rc/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesApps.java
Outdated
Show resolved
Hide resolved
...va/org/apache/hadoop/yarn/server/resourcemanager/webapp/helper/AppInfoJsonVerifications.java
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
- replace expected string to const - remove dead code
brumi1024
approved these changes
Oct 18, 2025
| YETUS='yetus' | ||
| // Branch or tag name. Yetus release tags are 'rel/X.Y.Z' | ||
| YETUS_VERSION='rel/0.14.0' | ||
| YETUS_VERSION='a7d29a6a72750a0c5c39512f33945e773e69303e' |
There was a problem hiding this comment.
Let's not forget to revert this in a follow-up, once this gets merged.
There was a problem hiding this comment.
Thanks @brumi1024 for the review!
|
@K0K0V0K Thank you for your contribution. I need some time to carefully review this PR and expect to provide a final conclusion by tomorrow. |
|
💔 -1 overall
This message was automatically generated. |
slfan1989
reviewed
Oct 19, 2025
...server-resourcemanager/src/test/java/org/apache/hadoop/yarn/webapp/TestRMWithCSRFFilter.java
Outdated
Show resolved
Hide resolved
slfan1989
reviewed
Oct 19, 2025
...doop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/webapp/JerseyTestBase.java
Outdated
Show resolved
Hide resolved
slfan1989
reviewed
Oct 19, 2025
...urcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/RMWebApp.java
Outdated
Show resolved
Hide resolved
slfan1989
reviewed
Oct 19, 2025
There was a problem hiding this comment.
I’ve checked out this PR locally and overall I agree with the changes — great work! There are just a few minor points I’d like to mention:
-
I’d prefer to keep the usage of
response.readEntity(JSONObject.class);.
Perhaps we could introduce an additional reader to support this, since this approach is actually the recommended one inJersey 2. -
The JSON comparison in the resource has been nicely restored to match the original one — really appreciate that!
However, for"attributes" : "", it would be better to change it to"attributes" : {}to maintain the correct structure. -
The
targetWithJsonObjectmethod in JerseyTestBase.java is also used in MapReduce and NodeManager, so it shouldn’t be replaced directly withtarget().
We need to make sure those modules remain compatible.
...a/org/apache/hadoop/yarn/server/resourcemanager/webapp/jsonprovider/JsonProviderFeature.java
Outdated
Show resolved
Hide resolved
...er-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestRMHA.java
Show resolved
Hide resolved
| "vCores" : 0, | ||
| "resourceInformations" : { | ||
| "resourceInformation" : [ { | ||
| "attributes" : "", |
There was a problem hiding this comment.
Rather than deleting "attributes": "", we should change it back to "attributes": { } to maintain the expected JSON format.
Hean-Chhinling
approved these changes
Oct 20, 2025
There was a problem hiding this comment.
LGTM with the latest changes!
Thank you @K0K0V0K for the contributions.
|
Thanks @slfan1989 for the review! I added an extra change, replaced the jersey-media-moxy with a native moxy dependency. Unfortunately i was not able to find solution for the |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Description of PR
JIRA: YARN-11874
Title: ResourceManager REST API backward compatibility with Jersey1
Background
During the upgrade from Jersey1 to Jersey2, the YARN REST API message structure changed in the following ways:
• The
javax.xml.bind.annotation.XmlTypeannotation is now generated as@xsi.typeinstead oftype.• JSON arrays with a single element are rendered as a single object rather than an array containing one object.
• Every XML root element is generated by default, whereas previously it was configurable.
These changes have caused compatibility issues:
• The YARN RM UI2 cannot load properly due to the changed API behavior.
• Some third-party systems that rely on the previous API behavior may break.
Solution
This PR reverts the API behavior to be compatible with Hadoop 3.4.2 (Jersey1) API documentation:
ResourceManager REST API (Hadoop 3.4.2)
Implementation Details
• It was not possible to configure jersey-media-json-jettison to behave like Jersey1.
• Introduced a new dependency: org.eclipse.persistence.moxy
• org.eclipse.persistence.moxy can be configured via the following properties:
MarshallerProperties Documentation
Testing