-
Notifications
You must be signed in to change notification settings - Fork 9.2k
YARN-11424. [Federation] Router Supports DeregisterSubCluster. #5363
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
Conversation
|
@goiri Can you help review this PR? Thank you very much! |
| /** Subcluster timeout allowed by Router. **/ | ||
| public static final String ROUTER_SUBCLUSTER_EXPIRATION_TIME = | ||
| ROUTER_PREFIX + "subcluster.heartbeat.expiration.time"; | ||
| public static final long DEFAULT_ROUTER_SUBCLUSTER_EXPIRATION_TIME = 1800000; |
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.
Make it TimeUnit
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.
Thank you very much for helping to review the code, I will modify the code.
| return; | ||
| } | ||
| if (usageInfo.args == null) { | ||
| builder.append("Usage: routeradmin [" + cmd + "]\n"); |
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.
If we have a StringBuilder, use append not +
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 will modify the code.
| } | ||
|
|
||
| public boolean isUsable() { | ||
| return !isUnusable(); |
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.
Let's remove isUnusable() and always use isUsable with ! and so on.
Negative methods are usually not recommended.
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 will fix it.
|
💔 -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. |
|
💔 -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. |
|
@goiri Can you help review this pr again? Thank you very much! |
...hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
Outdated
Show resolved
Hide resolved
|
|
||
| private DeregisterSubClusterResponse generateAllSubClusterData() { | ||
| List<DeregisterSubClusters> deregisterSubClusterList = new ArrayList<>(); | ||
| for (int i = 1; i <= 4; i++) { |
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.
Constant
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 will modify the code.
|
💔 -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. |
|
@goiri Can you help to merge this pr into the trunk branch? Thank you very much! I will continue to improve |
|
Can we get a clean build? |
I will retrigger Jenkins. |
|
💔 -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. |
|
@goiri Thank you very much for your help in reviewing the code! The javac compilation error is because we have changed the proto file. This part of the warning seems unavoidable. DeregisterSubClustersProto |
No way we can get a clean build then? |
|
@goiri This pr may not be able to clean bulid, I read this part of the code carefully. The javac warning should be caused by a new proto code added to this pr. This code will be directly compiled into We can check the following issue, the current version of protobuf does not seem to be able to solve it. In the complete compilation results of the We can see the link below: |
|
💔 -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. |
|
💔 -1 overall
This message was automatically generated. |
2113f77 to
c84c007
Compare
|
💔 -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. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
@goiri Can you help to merge this pr into the trunk branch? Thank you very much! I will continue to follow up YARN-11483. |
|
@goiri Thank you very much for helping to review the code! |
JIRA: YARN-11424. [Federation] Router Supports DeregisterSubCluster.
In YARN Federation mode, if we want to execute SubCluster commands, we need to go to SubCluster RM and execute commands.Router Should Provide Cli commands, So that We Can Execute Commands For Different SubClusters.
Router provides commands for Deregister SubCluster.