You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/02/01 17:09:21 UTC
[GitHub] [hadoop-ozone] timmylicheng opened a new pull request #516:
HDDS-2923 Add fall-back protection for rack awareness in pipeline creation.
timmylicheng opened a new pull request #516: HDDS-2923 Add fall-back protection for rack awareness in pipeline creation.
URL: https://github.com/apache/hadoop-ozone/pull/516
## What changes were proposed in this pull request?
#HDDS-2923 Add fall-back protection for rack awareness in pipeline creation.
(Please fill in changes proposed in this fix)
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-2923
(Please create an issue in ASF JIRA before opening a pull request,
and you need to set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. HDDS-XXXX. Fix a typo in YYY.)
Please replace this section with the link to the Apache JIRA)
## How was this patch tested?
UT. Test in actual env.
(Please explain how this patch was tested. Ex: unit tests, manual tests)
(If this patch involves UI changes, please attach a screen-shot; otherwise, remove this)
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] ChenSammi commented on a change in pull request
#516: HDDS-2923 Add fall-back protection for rack awareness in pipeline
creation.
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #516: HDDS-2923 Add fall-back protection for rack awareness in pipeline creation.
URL: https://github.com/apache/hadoop-ozone/pull/516#discussion_r376760574
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java
##########
@@ -220,50 +243,57 @@ boolean meetCriteria(DatanodeDetails datanodeDetails, int nodesRequired) {
public List<DatanodeDetails> getResultSet(
int nodesRequired, List<DatanodeDetails> healthyNodes)
throws SCMException {
+ if (nodesRequired != HddsProtos.ReplicationFactor.THREE.getNumber()) {
+ throw new SCMException("Nodes required number is not supported: " +
+ nodesRequired, SCMException.ResultCodes.INVALID_CAPACITY);
+ }
+
+ // Assume rack awareness is not enabled.
+ boolean rackAwareness = false;
List <DatanodeDetails> results = new ArrayList<>(nodesRequired);
// Since nodes are widely distributed, the results should be selected
// base on distance in topology, rack awareness and load balancing.
List<DatanodeDetails> exclude = new ArrayList<>();
// First choose an anchor nodes randomly
DatanodeDetails anchor = chooseNode(healthyNodes);
- if (anchor == null) {
- LOG.warn("Unable to find healthy node for anchor(first) node." +
- " Required nodes: {}, Found nodes: {}",
- nodesRequired, results.size());
- throw new SCMException("Unable to find required number of nodes.",
+ if (anchor != null) {
+ results.add(anchor);
+ exclude.add(anchor);
+ } else {
+ LOG.warn("Unable to find healthy node for anchor(first) node.");
+ throw new SCMException("Unable to find anchor node.",
SCMException.ResultCodes.FAILED_TO_FIND_SUITABLE_NODE);
}
if (LOG.isDebugEnabled()) {
LOG.debug("First node chosen: {}", anchor);
}
- results.add(anchor);
- exclude.add(anchor);
// Choose the second node on different racks from anchor.
- DatanodeDetails nodeOnDifferentRack = chooseNodeBasedOnRackAwareness(
+ DatanodeDetails nextNode = chooseNodeBasedOnRackAwareness(
healthyNodes, exclude,
nodeManager.getClusterNetworkTopologyMap(), anchor);
- if (nodeOnDifferentRack == null) {
- LOG.warn("Pipeline Placement: Unable to find 2nd node on different " +
- "racks that meets the criteria. Required nodes: {}, Found nodes:" +
- " {}", nodesRequired, results.size());
- throw new SCMException("Unable to find required number of nodes.",
- SCMException.ResultCodes.FAILED_TO_FIND_SUITABLE_NODE);
+ if (nextNode != null) {
+ // Rack awareness is detected.
+ rackAwareness = true;
+ results.add(nextNode);
+ exclude.add(nextNode);
+ } else {
+ LOG.debug("Pipeline Placement: Unable to find 2nd node on different " +
+ "rack based on rack awareness.");
}
if (LOG.isDebugEnabled()) {
Review comment:
Better to move this if statement into if (nextNode != null).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] timmylicheng commented on issue #516: HDDS-2923 Add
fall-back protection for rack awareness in pipeline creation.
Posted by GitBox <gi...@apache.org>.
timmylicheng commented on issue #516: HDDS-2923 Add fall-back protection for rack awareness in pipeline creation.
URL: https://github.com/apache/hadoop-ozone/pull/516#issuecomment-583271931
Acceptance test failed due to:
ERROR: Test execution of /home/runner/work/hadoop-ozone/hadoop-ozone/hadoop-ozone/dist/target/ozone-0.5.0-SNAPSHtOT/compose/ozonesecure is FAILED!!!!
Doesn't seem to be related to this change. @xiaoyuyao Do you have any ideas on the ozonesecure test failure? Thanks.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#516: HDDS-2923 Add fall-back protection for rack awareness in pipeline
creation.
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #516: HDDS-2923 Add fall-back protection for rack awareness in pipeline creation.
URL: https://github.com/apache/hadoop-ozone/pull/516#discussion_r373861618
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java
##########
@@ -241,29 +269,30 @@ boolean meetCriteria(DatanodeDetails datanodeDetails, int nodesRequired) {
exclude.add(anchor);
// Choose the second node on different racks from anchor.
- DatanodeDetails nodeOnDifferentRack = chooseNodeBasedOnRackAwareness(
+ DatanodeDetails nextNode = chooseNodeBasedOnRackAwareness(
healthyNodes, exclude,
nodeManager.getClusterNetworkTopologyMap(), anchor);
- if (nodeOnDifferentRack == null) {
- LOG.warn("Pipeline Placement: Unable to find 2nd node on different " +
- "racks that meets the criteria. Required nodes: {}, Found nodes:" +
- " {}", nodesRequired, results.size());
- throw new SCMException("Unable to find required number of nodes.",
- SCMException.ResultCodes.FAILED_TO_FIND_SUITABLE_NODE);
+ if (nextNode == null) {
Review comment:
NIT: I think we can change to check positive instead and let the for loop handle the remaining node picking naturally based on the results set size.
```
boolean rackAwareness = false;
if (nextNode != null) {
rackAwareness = true;
results.add(nextNode);
exclude.add(nextNode);
}
```
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] ChenSammi merged pull request #516: HDDS-2923 Add
fall-back protection for rack awareness in pipeline creation.
Posted by GitBox <gi...@apache.org>.
ChenSammi merged pull request #516: HDDS-2923 Add fall-back protection for rack awareness in pipeline creation.
URL: https://github.com/apache/hadoop-ozone/pull/516
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] timmylicheng commented on a change in pull request
#516: HDDS-2923 Add fall-back protection for rack awareness in pipeline
creation.
Posted by GitBox <gi...@apache.org>.
timmylicheng commented on a change in pull request #516: HDDS-2923 Add fall-back protection for rack awareness in pipeline creation.
URL: https://github.com/apache/hadoop-ozone/pull/516#discussion_r376770889
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java
##########
@@ -220,50 +243,57 @@ boolean meetCriteria(DatanodeDetails datanodeDetails, int nodesRequired) {
public List<DatanodeDetails> getResultSet(
int nodesRequired, List<DatanodeDetails> healthyNodes)
throws SCMException {
+ if (nodesRequired != HddsProtos.ReplicationFactor.THREE.getNumber()) {
+ throw new SCMException("Nodes required number is not supported: " +
+ nodesRequired, SCMException.ResultCodes.INVALID_CAPACITY);
+ }
+
+ // Assume rack awareness is not enabled.
+ boolean rackAwareness = false;
List <DatanodeDetails> results = new ArrayList<>(nodesRequired);
// Since nodes are widely distributed, the results should be selected
// base on distance in topology, rack awareness and load balancing.
List<DatanodeDetails> exclude = new ArrayList<>();
// First choose an anchor nodes randomly
DatanodeDetails anchor = chooseNode(healthyNodes);
- if (anchor == null) {
- LOG.warn("Unable to find healthy node for anchor(first) node." +
- " Required nodes: {}, Found nodes: {}",
- nodesRequired, results.size());
- throw new SCMException("Unable to find required number of nodes.",
+ if (anchor != null) {
+ results.add(anchor);
+ exclude.add(anchor);
+ } else {
+ LOG.warn("Unable to find healthy node for anchor(first) node.");
+ throw new SCMException("Unable to find anchor node.",
SCMException.ResultCodes.FAILED_TO_FIND_SUITABLE_NODE);
}
if (LOG.isDebugEnabled()) {
LOG.debug("First node chosen: {}", anchor);
}
- results.add(anchor);
- exclude.add(anchor);
// Choose the second node on different racks from anchor.
- DatanodeDetails nodeOnDifferentRack = chooseNodeBasedOnRackAwareness(
+ DatanodeDetails nextNode = chooseNodeBasedOnRackAwareness(
healthyNodes, exclude,
nodeManager.getClusterNetworkTopologyMap(), anchor);
- if (nodeOnDifferentRack == null) {
- LOG.warn("Pipeline Placement: Unable to find 2nd node on different " +
- "racks that meets the criteria. Required nodes: {}, Found nodes:" +
- " {}", nodesRequired, results.size());
- throw new SCMException("Unable to find required number of nodes.",
- SCMException.ResultCodes.FAILED_TO_FIND_SUITABLE_NODE);
+ if (nextNode != null) {
+ // Rack awareness is detected.
+ rackAwareness = true;
+ results.add(nextNode);
+ exclude.add(nextNode);
+ } else {
+ LOG.debug("Pipeline Placement: Unable to find 2nd node on different " +
+ "rack based on rack awareness.");
}
if (LOG.isDebugEnabled()) {
Review comment:
Sure. Updated.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] timmylicheng commented on a change in pull request
#516: HDDS-2923 Add fall-back protection for rack awareness in pipeline
creation.
Posted by GitBox <gi...@apache.org>.
timmylicheng commented on a change in pull request #516: HDDS-2923 Add fall-back protection for rack awareness in pipeline creation.
URL: https://github.com/apache/hadoop-ozone/pull/516#discussion_r375629748
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java
##########
@@ -241,29 +269,30 @@ boolean meetCriteria(DatanodeDetails datanodeDetails, int nodesRequired) {
exclude.add(anchor);
// Choose the second node on different racks from anchor.
- DatanodeDetails nodeOnDifferentRack = chooseNodeBasedOnRackAwareness(
+ DatanodeDetails nextNode = chooseNodeBasedOnRackAwareness(
healthyNodes, exclude,
nodeManager.getClusterNetworkTopologyMap(), anchor);
- if (nodeOnDifferentRack == null) {
- LOG.warn("Pipeline Placement: Unable to find 2nd node on different " +
- "racks that meets the criteria. Required nodes: {}, Found nodes:" +
- " {}", nodesRequired, results.size());
- throw new SCMException("Unable to find required number of nodes.",
- SCMException.ResultCodes.FAILED_TO_FIND_SUITABLE_NODE);
+ if (nextNode == null) {
Review comment:
Updated.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#516: HDDS-2923 Add fall-back protection for rack awareness in pipeline
creation.
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #516: HDDS-2923 Add fall-back protection for rack awareness in pipeline creation.
URL: https://github.com/apache/hadoop-ozone/pull/516#discussion_r373861618
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java
##########
@@ -241,29 +269,30 @@ boolean meetCriteria(DatanodeDetails datanodeDetails, int nodesRequired) {
exclude.add(anchor);
// Choose the second node on different racks from anchor.
- DatanodeDetails nodeOnDifferentRack = chooseNodeBasedOnRackAwareness(
+ DatanodeDetails nextNode = chooseNodeBasedOnRackAwareness(
healthyNodes, exclude,
nodeManager.getClusterNetworkTopologyMap(), anchor);
- if (nodeOnDifferentRack == null) {
- LOG.warn("Pipeline Placement: Unable to find 2nd node on different " +
- "racks that meets the criteria. Required nodes: {}, Found nodes:" +
- " {}", nodesRequired, results.size());
- throw new SCMException("Unable to find required number of nodes.",
- SCMException.ResultCodes.FAILED_TO_FIND_SUITABLE_NODE);
+ if (nextNode == null) {
Review comment:
NIT: I think we can change to check positive instead and let the for loop handle the remaining node picking naturally.
```
boolean rackAwareness = false;
if (nextNode != null) {
rackAwareness = true;
results.add(nextNode);
exclude.add(nextNode);
}
```
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#516: HDDS-2923 Add fall-back protection for rack awareness in pipeline
creation.
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #516: HDDS-2923 Add fall-back protection for rack awareness in pipeline creation.
URL: https://github.com/apache/hadoop-ozone/pull/516#discussion_r373861794
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelinePlacementPolicy.java
##########
@@ -83,10 +83,36 @@ public void testChooseNodeBasedOnRackAwareness() {
DatanodeDetails nextNode = placementPolicy.chooseNodeBasedOnRackAwareness(
healthyNodes, new ArrayList<>(PIPELINE_PLACEMENT_MAX_NODES_COUNT),
topologyWithDifRacks, anchor);
+ Assert.assertNotNull(nextNode);
Assert.assertFalse(anchor.getNetworkLocation().equals(
nextNode.getNetworkLocation()));
}
+ @Test
Review comment:
can we add a test simple test case with topology that all nodes on the same rack with topology aware and fall back enabled?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] ChenSammi commented on issue #516: HDDS-2923 Add
fall-back protection for rack awareness in pipeline creation.
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on issue #516: HDDS-2923 Add fall-back protection for rack awareness in pipeline creation.
URL: https://github.com/apache/hadoop-ozone/pull/516#issuecomment-583919237
+1
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] timmylicheng commented on a change in pull request
#516: HDDS-2923 Add fall-back protection for rack awareness in pipeline
creation.
Posted by GitBox <gi...@apache.org>.
timmylicheng commented on a change in pull request #516: HDDS-2923 Add fall-back protection for rack awareness in pipeline creation.
URL: https://github.com/apache/hadoop-ozone/pull/516#discussion_r375629800
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelinePlacementPolicy.java
##########
@@ -83,10 +83,36 @@ public void testChooseNodeBasedOnRackAwareness() {
DatanodeDetails nextNode = placementPolicy.chooseNodeBasedOnRackAwareness(
healthyNodes, new ArrayList<>(PIPELINE_PLACEMENT_MAX_NODES_COUNT),
topologyWithDifRacks, anchor);
+ Assert.assertNotNull(nextNode);
Assert.assertFalse(anchor.getNetworkLocation().equals(
nextNode.getNetworkLocation()));
}
+ @Test
Review comment:
I add a test. Please check the latest commit.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org