[
https://issues.apache.org/jira/browse/HDFS-16767?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17603701#comment-17603701
]
ASF GitHub Bot commented on HDFS-16767:
---------------------------------------
omalley commented on code in PR #4127:
URL: https://github.com/apache/hadoop/pull/4127#discussion_r969937548
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java:
##########
@@ -200,6 +207,17 @@ public RouterRpcClient(Configuration conf, Router router,
failoverSleepBaseMillis, failoverSleepMaxMillis);
String[] ipProxyUsers = conf.getStrings(DFS_NAMENODE_IP_PROXY_USERS);
this.enableProxyUser = ipProxyUsers != null && ipProxyUsers.length > 0;
+ this.observerReadEnabled = conf.getBoolean(
+ RBFConfigKeys.DFS_ROUTER_OBSERVER_READ_ENABLE,
+ RBFConfigKeys.DFS_ROUTER_OBSERVER_READ_ENABLE_DEFAULT);
+ Map<String, String> observerReadOverrides = conf
+ .getPropsWithPrefix(RBFConfigKeys.DFS_ROUTER_OBSERVER_READ_ENABLE +
".");
Review Comment:
I'd rather have this be a property that inverts the default with a name like
FEDERATION_ROUTER_PREFIX + ".observer.read.overrides" that is a list of
namespaces that are not the default. It will be easier to configure with a
single property and it can be in RBFConfigKeys.
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MembershipNamenodeResolver.java:
##########
@@ -73,8 +74,13 @@
/** Parent router ID. */
private String routerId;
- /** Cached lookup of NN for nameservice. Invalidated on cache refresh. */
+ /** Cached lookup of NN for nameservice with active state ranked first.
Review Comment:
Rather than having a second map, I'd suggest adding the useObserver as part
of the key and making it: Map<Pair<String,Boolean>,List<...>>
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcServer.java:
##########
@@ -337,18 +337,17 @@ public RouterRpcServer(Configuration conf, Router router,
.build();
// Add all the RPC protocols that the Router implements
- DFSUtil.addPBProtocol(
- conf, NamenodeProtocolPB.class, nnPbService, this.rpcServer);
- DFSUtil.addPBProtocol(conf, RefreshUserMappingsProtocolPB.class,
+ DFSUtil.addPBProtocol(this.conf, NamenodeProtocolPB.class, nnPbService,
this.rpcServer);
+ DFSUtil.addPBProtocol(this.conf, RefreshUserMappingsProtocolPB.class,
refreshUserMappingService, this.rpcServer);
- DFSUtil.addPBProtocol(conf, GetUserMappingsProtocolPB.class,
+ DFSUtil.addPBProtocol(this.conf, GetUserMappingsProtocolPB.class,
Review Comment:
Do we need to make the change from "conf" to "this.conf"?
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterRpcClient.java:
##########
@@ -462,7 +481,8 @@ private RetryDecision shouldRetry(final IOException ioe,
final int retryCount,
public Object invokeMethod(
final UserGroupInformation ugi,
final List<? extends FederationNamenodeContext> namenodes,
- final Class<?> protocol, final Method method, final Object... params)
+ final Class<?> protocol, final Method method, boolean skipObserver,
Review Comment:
It's a nit, but I'd move skipObserver up next to the namenodes. At the
higher level we use useObserver and negate when calling here. We should
probably be consistent.
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RBFConfigKeys.java:
##########
@@ -191,6 +191,10 @@ public class RBFConfigKeys extends
CommonConfigurationKeysPublic {
FEDERATION_STORE_PREFIX + "enable";
public static final boolean DFS_ROUTER_STORE_ENABLE_DEFAULT = true;
+ public static final String DFS_ROUTER_OBSERVER_READ_ENABLE =
Review Comment:
This is really the default of whether observers are enabled, so I'd propose:
DFS_ROUTER_OBSERVER_READ =
FEDERATION_ROUTER_PREFIX + ".observer.read.default"
> RBF: Support observer node from Router-Based Federation
> --------------------------------------------------------
>
> Key: HDFS-16767
> URL: https://issues.apache.org/jira/browse/HDFS-16767
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Reporter: Simbarashe Dzinamarira
> Assignee: Simbarashe Dzinamarira
> Priority: Major
> Labels: pull-request-available
>
> Enable routers to direct read calls to observer namenodes.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
