[GitHub] [hadoop] xiaoyuyao commented on a change in pull request #931: HDDS-1586. Allow Ozone RPC client to read with topology awareness.
xiaoyuyao commented on a change in pull request #931: HDDS-1586. Allow Ozone
RPC client to read with topology awareness.
URL: https://github.com/apache/hadoop/pull/931#discussion_r301683523
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##
@@ -2347,7 +2347,7 @@ public OmKeyInfo lookupKey(OmKeyArgs args) throws
IOException {
boolean auditSuccess = true;
try {
metrics.incNumKeyLookups();
- return keyManager.lookupKey(args);
+ return keyManager.lookupKey(args, getClientAddress());
Review comment:
Do we assume the topology mapping always use Ip address?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] xiaoyuyao commented on a change in pull request #931: HDDS-1586. Allow Ozone RPC client to read with topology awareness.
xiaoyuyao commented on a change in pull request #931: HDDS-1586. Allow Ozone RPC client to read with topology awareness. URL: https://github.com/apache/hadoop/pull/931#discussion_r293472081 ## File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfigKeys.java ## @@ -363,6 +363,10 @@ "ozone.scm.network.topology.schema.file"; public static final String OZONE_SCM_NETWORK_TOPOLOGY_SCHEMA_FILE_DEFAULT = "network-topology-default.xml"; + public static final String DFS_NETWORK_TOPOLOGY_AWARE_READ_ENABLED = + "dfs.network.topology.aware.read.enable"; + public static final String DFS_NETWORK_TOPOLOGY_AWARE_READ_ENABLED_DEFAULT = + "true"; Review comment: Not at this time. Let's keep it as-is and revisit if we need to make this a server side option. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] xiaoyuyao commented on a change in pull request #931: HDDS-1586. Allow Ozone RPC client to read with topology awareness.
xiaoyuyao commented on a change in pull request #931: HDDS-1586. Allow Ozone
RPC client to read with topology awareness.
URL: https://github.com/apache/hadoop/pull/931#discussion_r293471766
##
File path:
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientManager.java
##
@@ -150,11 +201,16 @@ public void releaseClient(XceiverClientSpi client,
boolean invalidateClient) {
}
}
- private XceiverClientSpi getClient(Pipeline pipeline)
+ private XceiverClientSpi getClient(Pipeline pipeline, boolean forRead)
throws IOException {
HddsProtos.ReplicationType type = pipeline.getType();
try {
+ // create different client for read different pipeline node based on
+ // network topology
String key = pipeline.getId().getId().toString() + type;
Review comment:
Can we wrap this logic in a help function like getPipelineKey()?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] xiaoyuyao commented on a change in pull request #931: HDDS-1586. Allow Ozone RPC client to read with topology awareness.
xiaoyuyao commented on a change in pull request #931: HDDS-1586. Allow Ozone
RPC client to read with topology awareness.
URL: https://github.com/apache/hadoop/pull/931#discussion_r293460005
##
File path:
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java
##
@@ -596,6 +620,81 @@ private OmKeyArgs createKeyArgs(String toKeyName) throws
IOException {
return createBuilder().setKeyName(toKeyName).build();
}
+ @Test
+ public void testLookupKeyWithLocation() throws IOException {
+String keyName = RandomStringUtils.randomAlphabetic(5);
+OmKeyArgs keyArgs = createBuilder()
+.setKeyName(keyName)
+.build();
+
+// lookup for a non-existent key
+try {
+ keyManager.lookupKey(keyArgs, null);
+ Assert.fail("Lookup key should fail for non existent key");
+} catch (OMException ex) {
+ if (ex.getResult() != OMException.ResultCodes.KEY_NOT_FOUND) {
+throw ex;
+ }
+}
+
+// create a key
+OpenKeySession keySession = keyManager.createFile(keyArgs, false, false);
+// randomly select 3 datanodes
+List nodeList = new ArrayList<>();
+nodeList.add((DatanodeDetails)scm.getClusterMap().getNode(
+0, null, null, null, null, 0));
+nodeList.add((DatanodeDetails)scm.getClusterMap().getNode(
+1, null, null, null, null, 0));
+nodeList.add((DatanodeDetails)scm.getClusterMap().getNode(
+2, null, null, null, null, 0));
+Assume.assumeTrue(nodeList.get(0) != nodeList.get(1));
Review comment:
should we use .equals() here?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] xiaoyuyao commented on a change in pull request #931: HDDS-1586. Allow Ozone RPC client to read with topology awareness.
xiaoyuyao commented on a change in pull request #931: HDDS-1586. Allow Ozone RPC client to read with topology awareness. URL: https://github.com/apache/hadoop/pull/931#discussion_r293206378 ## File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfigKeys.java ## @@ -363,6 +363,10 @@ "ozone.scm.network.topology.schema.file"; public static final String OZONE_SCM_NETWORK_TOPOLOGY_SCHEMA_FILE_DEFAULT = "network-topology-default.xml"; + public static final String DFS_NETWORK_TOPOLOGY_AWARE_READ_ENABLED = + "dfs.network.topology.aware.read.enable"; + public static final String DFS_NETWORK_TOPOLOGY_AWARE_READ_ENABLED_DEFAULT = + "true"; Review comment: should we set the default to false so that if the deployment does not config a topology we don't have the overhead. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] xiaoyuyao commented on a change in pull request #931: HDDS-1586. Allow Ozone RPC client to read with topology awareness.
xiaoyuyao commented on a change in pull request #931: HDDS-1586. Allow Ozone
RPC client to read with topology awareness.
URL: https://github.com/apache/hadoop/pull/931#discussion_r293205523
##
File path:
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientManager.java
##
@@ -76,7 +81,8 @@ public XceiverClientManager(Configuration conf) {
SCM_CONTAINER_CLIENT_MAX_SIZE_DEFAULT);
long staleThresholdMs = conf.getTimeDuration(
SCM_CONTAINER_CLIENT_STALE_THRESHOLD_KEY,
-SCM_CONTAINER_CLIENT_STALE_THRESHOLD_DEFAULT, TimeUnit.MILLISECONDS);
+//SCM_CONTAINER_CLIENT_STALE_THRESHOLD_DEFAULT, TimeUnit.MILLISECONDS);
Review comment:
Is there a reason to hard code the stale threshold here?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
