[GitHub] [hadoop] tomscut commented on a diff in pull request #4127: HDFS-13522. RBF: Support observer node from Router-Based Federation
tomscut commented on code in PR #4127: URL: https://github.com/apache/hadoop/pull/4127#discussion_r847843114 ## hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java: ## @@ -1380,8 +1437,9 @@ private static boolean isExpectedValue(Object expectedValue, Object value) { final CallerContext originContext = CallerContext.getCurrent(); for (final T location : locations) { String nsId = location.getNameserviceId(); + boolean isObserverRead = observerReadEnabled && isReadCall(m); final List namenodes = - getNamenodesForNameservice(nsId); + msync(nsId, ugi, isObserverRead); Review Comment: > @tomscut for "Here's how we do it.", is there a link you meant to attach? I mean, in our cluster, we ran into this problem, and this is how we solved it. ## hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/HdfsClientConfigKeys.java: ## @@ -79,6 +79,8 @@ String DFS_NAMENODE_HTTPS_ADDRESS_KEY = "dfs.namenode.https-address"; String DFS_HA_NAMENODES_KEY_PREFIX = "dfs.ha.namenodes"; int DFS_NAMENODE_RPC_PORT_DEFAULT = 8020; + String DFS_OBSERVER_READ_ENABLE = "dfs.observer.read.enable"; + boolean DFS_OBSERVER_READ_ENABLE_DEFAULT = true; Review Comment: Thank you for your explanation. I understand. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] tomscut commented on a diff in pull request #4127: HDFS-13522. RBF: Support observer node from Router-Based Federation
tomscut commented on code in PR #4127: URL: https://github.com/apache/hadoop/pull/4127#discussion_r845916975 ## hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java: ## @@ -1380,8 +1437,9 @@ private static boolean isExpectedValue(Object expectedValue, Object value) { final CallerContext originContext = CallerContext.getCurrent(); for (final T location : locations) { String nsId = location.getNameserviceId(); + boolean isObserverRead = observerReadEnabled && isReadCall(m); final List namenodes = - getNamenodesForNameservice(nsId); + msync(nsId, ugi, isObserverRead); Review Comment: If `observerReadEnabled` is a global config, assume that there are two NS. NS1 supports msync, but NS2 does not. If NS1 is executed, a `NoSuchMethodException` may be thrown, causing the entire request to fail. Maybe we should also add a NS level config, such as `nsToObserverReadEnabled`. Here's how we do it. ## hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/HdfsClientConfigKeys.java: ## @@ -79,6 +79,8 @@ String DFS_NAMENODE_HTTPS_ADDRESS_KEY = "dfs.namenode.https-address"; String DFS_HA_NAMENODES_KEY_PREFIX = "dfs.ha.namenodes"; int DFS_NAMENODE_RPC_PORT_DEFAULT = 8020; + String DFS_OBSERVER_READ_ENABLE = "dfs.observer.read.enable"; + boolean DFS_OBSERVER_READ_ENABLE_DEFAULT = true; Review Comment: Hi @simbadzina , should the default here be false? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org