-
Notifications
You must be signed in to change notification settings - Fork 8.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
HDFS-16411 RBF: RouterId is NULL when disable RourterRpcServer #3878
Conversation
💔 -1 overall
This message was automatically generated. |
…nable=false; modify TestRouterHeartbeatService
💔 -1 overall
This message was automatically generated. |
...adoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouter.java
Show resolved
Hide resolved
…nable=false; modify testSwitchRouter()
💔 -1 overall
This message was automatically generated. |
…nable=false; checkstyle
💔 -1 overall
This message was automatically generated. |
assertRouterHeartbeater(true, false); | ||
assertRouterHeartbeater(false, true); | ||
assertRouterHeartbeater(false, false); | ||
assertRouterHeartbeater(true,true, true); |
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.
Watch the spaces after the comma.
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.
Thanks for your advice. In the latest commit e208d71 , it has been done.
assertRouterHeartbeater(true,true, true); | |
assertRouterHeartbeater(true, true, true); |
RBFConfigKeys.DFS_ROUTER_RPC_ADDRESS_DEFAULT, | ||
RBFConfigKeys.DFS_ROUTER_RPC_PORT_DEFAULT); | ||
|
||
String hostname = InetAddress.getLocalHost().getHostName(); |
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.
Most of this is already defined in setRpcServerAddress() and it even considers exception, etc.
I would refactor that function to allow specifying a customized port.
In any case, I would leave the general code as it was, and then after that, check if routerId is null, and then set it to this.
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.
Thanks for your advice. The method setRpcServerAddress() is only called when rpc.enable is true. Maybe we can add a new method after that and check routerid.
@@ -76,6 +75,8 @@ public void setup() throws Exception { | |||
curatorFramework.start(); | |||
routerConfig.set(CommonConfigurationKeys.ZK_ADDRESS, connectStr); | |||
router.init(routerConfig); | |||
// set custom routerid after router.init() | |||
router.setRouterId(routerId); |
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.
Wouldnt init already trigger this with the new code change?
Or we need to force the specific routerId?
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 set the specific routerid,we use the specific routerid; otherwise we use the routerid generated by Router.serviceInit.
…nable=false; checkstyle
…nable=false; checkstyle
💔 -1 overall
This message was automatically generated. |
...ct/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/Router.java
Outdated
Show resolved
Hide resolved
...ct/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/Router.java
Show resolved
Hide resolved
...ct/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/Router.java
Show resolved
Hide resolved
…nable=false; checkstyle
💔 -1 overall
This message was automatically generated. |
…nable=false; checkstyle
…nable=false; checkstyle
💔 -1 overall
This message was automatically generated. |
TestRouterDistCpProcedure seems to be failing consistently. |
Yeps, have seen this failing quite often, mostly a timeout issue, increasing the timeout in the test should fix it, I think so. Not sure why but our builds are getting slower and slower... |
@goiri Whether we continue this PR,or close it |
According to @ayushtkn, the failures are not introduced by this work so let's go with it. |
JIRA: HDFS-16411
When dfs.federation.router.rpc.enable=false, routerid is null, but RouterHeartbeatService need updateStateStore() with routerId.