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 2021/06/21 23:26:35 UTC
[GitHub] [ozone] errose28 opened a new pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
errose28 opened a new pull request #2354:
URL: https://github.com/apache/ozone/pull/2354
## What changes were proposed in this pull request?
HDDS-5252 added a check that datanodes have enough space for Ratis factor three pipelines. This PR adds this check for Ratis factor one pipelines as well. These factor one pipelines could be auto created based on the configuration, or written to using the replication factor command line flag.
Some refactoring of the original approach was done to make this easier to follow IMHO. Previously the existing `sizeRequired` parameter of `PlacementPolicy#chooseDatanodes` was ignored in pipeline policies, and only used by container placement policies. Since we are now placing a size requirement on pipeline creation, it makes sense to use this existing parameter instead of always passing 0 and having the implementation ignore the value while pulling a separate value from the configuration.
## What is the link to the Apache JIRA
HDDS-5337
## How was this patch tested?
Unit test added. Existing unit test 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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ChenSammi commented on pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#issuecomment-881126880
The last patch LGTM，+1.
Thanks @errose28 for the contribution and @bshashikant @guihecheng for the code review.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] errose28 commented on a change in pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
errose28 commented on a change in pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#discussion_r670140395
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java
##########
@@ -196,7 +200,7 @@ public boolean hasEnoughSpace(DatanodeDetails datanodeDetails,
for (MetadataStorageReportProto reportProto
: datanodeInfo.getMetadataStorageReports()) {
- if (reportProto.getRemaining() > metaSizeRequired) {
+ if (reportProto.getRemaining() > metadataSizeRequired) {
Review comment:
Thanks @ChenSammi I added the check and addressed some minor test failures.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] guihecheng commented on a change in pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
guihecheng commented on a change in pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#discussion_r655824142
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineProvider.java
##########
@@ -26,16 +26,20 @@
import org.apache.hadoop.hdds.client.ReplicationConfig;
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.scm.SCMCommonPlacementPolicy;
import org.apache.hadoop.hdds.scm.exceptions.SCMException;
import org.apache.hadoop.hdds.scm.node.NodeManager;
import org.apache.hadoop.hdds.scm.node.NodeStatus;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* Interface for creating pipelines.
*/
public abstract class PipelineProvider<REPLICATION_CONFIG
extends ReplicationConfig> {
-
+ private static final Logger LOG =
+ LoggerFactory.getLogger(SCMCommonPlacementPolicy.class);
Review comment:
A copy-paste error? Normally we have the classname 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] errose28 commented on a change in pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
errose28 commented on a change in pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#discussion_r656408607
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelinePlacementPolicy.java
##########
@@ -195,9 +200,11 @@ public void testChooseNodeNotEnoughSpace() throws SCMException {
int nodesRequired = HddsProtos.ReplicationFactor.THREE.getNumber();
thrownExp.expect(SCMException.class);
- thrownExp.expectMessage("enough space for even a single container");
+ thrownExp.expectMessage("Unable to find enough nodes that meet" +
+ "the space requirement");
Review comment:
Thanks, I wrapped the line for checkstyle and did not re-run the test after.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] errose28 commented on pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
errose28 commented on pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#issuecomment-867338788
Thanks @ChenSammi I will take a look at that patch and incorporate the factor 1 part here too.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] errose28 commented on pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
errose28 commented on pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#issuecomment-881145872
Thanks @ChenSammi @bshashikant and @guihecheng for reviews!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] errose28 commented on pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
errose28 commented on pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#issuecomment-865409993
cc @guihecheng for review
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] errose28 commented on pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
errose28 commented on pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#issuecomment-866162336
Thanks for the review @guihecheng, I've updated the code accordingly.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] guihecheng commented on pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
guihecheng commented on pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#issuecomment-865469140
@errose28 Thanks very much for fixing this, the idea is good that we have the container size from conf as an argument passed to those pick/choose functions, actually there are similar thoughts during the discussion in the pr mentioned above.
Only some inline comments above.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] guihecheng commented on pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
guihecheng commented on pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#issuecomment-876091867
> Sorry for the delays in updating this patch. Just got back from numerous company holidays. In addition to adding metadata checks for factor one pipelines, I fixed the code from the previous Jiras so that closed container replication should now only require the size of the container being replicated to be available on a datanode (it does not need ratis metadata and may not need a full container size worth of space). See how `ReplicationManager#handleUnderReplicatedContainer` uses the new code. Unfortunately adding the metadata size parameter resulted in small changes to many files.
Thanks @errose28 , I think the idea to remove the limit for replication on metadata size is good, and the added parameter serves the purpose well.
Verified manually on the Ratis factor one case, it works well.
And the overall patches LGTM.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] errose28 commented on a change in pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
errose28 commented on a change in pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#discussion_r656407862
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineProvider.java
##########
@@ -26,16 +26,20 @@
import org.apache.hadoop.hdds.client.ReplicationConfig;
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.scm.SCMCommonPlacementPolicy;
import org.apache.hadoop.hdds.scm.exceptions.SCMException;
import org.apache.hadoop.hdds.scm.node.NodeManager;
import org.apache.hadoop.hdds.scm.node.NodeStatus;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* Interface for creating pipelines.
*/
public abstract class PipelineProvider<REPLICATION_CONFIG
extends ReplicationConfig> {
-
+ private static final Logger LOG =
+ LoggerFactory.getLogger(SCMCommonPlacementPolicy.class);
Review comment:
Oops yes I make this mistake all the time. 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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] errose28 commented on pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
errose28 commented on pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#issuecomment-868213307
Thanks @bshashikant. Rebase involves incorporating changes @ChenSammi mentioned above, I will try to get to this tomorrow so the patch addresses both your comments.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] errose28 commented on pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
errose28 commented on pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#issuecomment-865409993
cc @guihecheng for review
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] errose28 commented on pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
errose28 commented on pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#issuecomment-875971142
Sorry for the delays in updating this patch. Just got back from numerous company holidays. In addition to adding metadata checks for factor one pipelines, I fixed the code from the previous Jiras so that closed container replication should now only require the size of the container being replicated to be available on a datanode (it does not need ratis metadata and may not need a full container size worth of space). See how `ReplicationManager#handleUnderReplicatedContainer` uses the new code. Unfortunately adding the metadata size parameter resulted in small changes to many files.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] guihecheng commented on a change in pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
guihecheng commented on a change in pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#discussion_r655824142
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineProvider.java
##########
@@ -26,16 +26,20 @@
import org.apache.hadoop.hdds.client.ReplicationConfig;
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.scm.SCMCommonPlacementPolicy;
import org.apache.hadoop.hdds.scm.exceptions.SCMException;
import org.apache.hadoop.hdds.scm.node.NodeManager;
import org.apache.hadoop.hdds.scm.node.NodeStatus;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* Interface for creating pipelines.
*/
public abstract class PipelineProvider<REPLICATION_CONFIG
extends ReplicationConfig> {
-
+ private static final Logger LOG =
+ LoggerFactory.getLogger(SCMCommonPlacementPolicy.class);
Review comment:
A copy-paste error? Normally we have the classname here.
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelinePlacementPolicy.java
##########
@@ -195,9 +200,11 @@ public void testChooseNodeNotEnoughSpace() throws SCMException {
int nodesRequired = HddsProtos.ReplicationFactor.THREE.getNumber();
thrownExp.expect(SCMException.class);
- thrownExp.expectMessage("enough space for even a single container");
+ thrownExp.expectMessage("Unable to find enough nodes that meet" +
+ "the space requirement");
Review comment:
Here should a single space, then the failed ut should pass I think.
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelinePlacementPolicy.java
##########
@@ -195,9 +200,11 @@ public void testChooseNodeNotEnoughSpace() throws SCMException {
int nodesRequired = HddsProtos.ReplicationFactor.THREE.getNumber();
thrownExp.expect(SCMException.class);
- thrownExp.expectMessage("enough space for even a single container");
+ thrownExp.expectMessage("Unable to find enough nodes that meet" +
+ "the space requirement");
Review comment:
Here should have a single space, then the failed ut should pass I think.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] guihecheng commented on a change in pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
guihecheng commented on a change in pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#discussion_r655824372
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelinePlacementPolicy.java
##########
@@ -195,9 +200,11 @@ public void testChooseNodeNotEnoughSpace() throws SCMException {
int nodesRequired = HddsProtos.ReplicationFactor.THREE.getNumber();
thrownExp.expect(SCMException.class);
- thrownExp.expectMessage("enough space for even a single container");
+ thrownExp.expectMessage("Unable to find enough nodes that meet" +
+ "the space requirement");
Review comment:
Here should a single space, then the failed ut should pass I think.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ChenSammi merged pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
ChenSammi merged pull request #2354:
URL: https://github.com/apache/ozone/pull/2354
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] bshashikant commented on pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
bshashikant commented on pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#issuecomment-868210224
@errose28 , can you please rebase. The patch looks good otherwise.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ChenSammi commented on a change in pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#discussion_r669362019
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java
##########
@@ -196,7 +200,7 @@ public boolean hasEnoughSpace(DatanodeDetails datanodeDetails,
for (MetadataStorageReportProto reportProto
: datanodeInfo.getMetadataStorageReports()) {
- if (reportProto.getRemaining() > metaSizeRequired) {
+ if (reportProto.getRemaining() > metadataSizeRequired) {
Review comment:
Can you add a “metadataSizeRequired not 0" if check around this for loop? since metadataSizeRequired is passed as 0 in container replication.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] guihecheng commented on pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
guihecheng commented on pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#issuecomment-865469140
@errose28 Thanks very much for fixing this, the idea is good that we have the container size from conf as an argument passed to those pick/choose functions, actually there are similar thoughts during the discussion in the pr mentioned above.
Only some inline comments above.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ChenSammi commented on a change in pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#discussion_r669362019
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java
##########
@@ -196,7 +200,7 @@ public boolean hasEnoughSpace(DatanodeDetails datanodeDetails,
for (MetadataStorageReportProto reportProto
: datanodeInfo.getMetadataStorageReports()) {
- if (reportProto.getRemaining() > metaSizeRequired) {
+ if (reportProto.getRemaining() > metadataSizeRequired) {
Review comment:
Can you add a “metadataSizeRequired not 0" if check around this for loop? since metadataSizeRequired is passed as 0 in container replication.
The remaining looks good. Just above the comment.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] guihecheng commented on a change in pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
guihecheng commented on a change in pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#discussion_r655824372
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelinePlacementPolicy.java
##########
@@ -195,9 +200,11 @@ public void testChooseNodeNotEnoughSpace() throws SCMException {
int nodesRequired = HddsProtos.ReplicationFactor.THREE.getNumber();
thrownExp.expect(SCMException.class);
- thrownExp.expectMessage("enough space for even a single container");
+ thrownExp.expectMessage("Unable to find enough nodes that meet" +
+ "the space requirement");
Review comment:
Here should have a single space, then the failed ut should pass I think.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ChenSammi commented on pull request #2354: HDDS-5337. Apply container space check to Ratis factor one pipelines
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on pull request #2354:
URL: https://github.com/apache/ozone/pull/2354#issuecomment-866571647
@errose28 ，thanks for take care of the factor one pipeline part. The current patch overall LGTM.
@guihecheng has another patch HDDS-5269 to exclude low storage ratis directory DN from pipeline creation.
Ethan, would you help to cover the factor one pipeline ratis part in this patch too after HDDS-5269 is merged?
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org