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/07/30 07:38:08 UTC
[GitHub] [hadoop-ozone] bshashikant opened a new pull request #1274: HDDS-3810. Add the logic to distribute open containers among the pipelines of a datanode.
bshashikant opened a new pull request #1274:
URL: https://github.com/apache/hadoop-ozone/pull/1274
## What changes were proposed in this pull request?
Added logic for open container distribution among multiple pipelines of a dn
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-3810
## How was this patch tested?
Added unit tests.
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] GlenGeng commented on a change in pull request #1274: HDDS-3810. Add the logic to distribute open containers among the pipelines of a datanode.
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1274:
URL: https://github.com/apache/hadoop-ozone/pull/1274#discussion_r496348325
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeInfo.java
##########
@@ -40,6 +43,8 @@
private long lastStatsUpdatedTime;
private List<StorageReportProto> storageReports;
+ private List<StorageContainerDatanodeProtocolProtos.
Review comment:
may replace `StorageContainerDatanodeProtocolProtos.MetadataStorageReportProto` with `MetadataStorageReportProto`
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeInfo.java
##########
@@ -94,6 +100,22 @@ public void updateStorageReports(List<StorageReportProto> reports) {
}
}
+ /**
+ * Updates the datanode storage reports.
Review comment:
stale java doc, suggestion: Updates the datanode metadata storage reports.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
##########
@@ -530,6 +540,43 @@ public int getNumHealthyVolumes(List<DatanodeDetails> dnList) {
return Collections.max(volumeCountList);
}
+ /**
+ * Returns the pipeline limit for the datanode.
+ * if the datanode pipeline limit is set, consider that as the max
+ * pipeline limit.
+ * In case, the pipeline limit is not set, the max pipeline limit
+ * will be based on the no of raft log volume reported and provided
+ * that it has atleast one healthy data volume.
+ */
+ @Override
+ public int maxPipelineLimit(DatanodeDetails dn) {
+ try {
+ if (heavyNodeCriteria > 0) {
+ return heavyNodeCriteria;
+ } else if (nodeStateManager.getNode(dn).getHealthyVolumeCount() > 0) {
+ return numPipelinesPerRaftLogDisk *
+ nodeStateManager.getNode(dn).getRaftLogVolumeCount();
+ }
+ } catch (NodeNotFoundException e) {
+ LOG.warn("Cannot generate NodeStat, datanode {} not found.",
+ dn.getUuid());
+ }
+ return 0;
+ }
+
+ /**
+ * Returns the pipeline limit for set of datanodes.
+ */
+ @Override
+ public int maxPipelineLimit(List<DatanodeDetails> dnList) {
Review comment:
method name is `maxPipelineLimit`, but the logic calculates the min, which is a little bit weird. How about `minPipelineLimit `
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfigKeys.java
##########
@@ -308,6 +308,10 @@
OZONE_SCM_KEY_VALUE_CONTAINER_DELETION_CHOOSING_POLICY =
"ozone.scm.keyvalue.container.deletion-choosing.policy";
+ public static final String OZONE_SCM_PIPELINE_PER_RAFT_LOG_DISK =
Review comment:
How about `OZONE_SCM_PIPELINE_PER_METADATA_DISK` ?
There are both raft log disk and meta data storage report existing in the code context, they are similar to each other, bring in some redundancy.
BTW, the former one may lead misunderstanding. One might think that each raft log disk contains one raft log, which is straightforwad, nevertheless, the raft log disk and the raft log is a OneToMany relationship.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/SCMContainerManager.java
##########
@@ -101,12 +103,23 @@ public SCMContainerManager(
this.numContainerPerVolume = conf
.getInt(ScmConfigKeys.OZONE_SCM_PIPELINE_OWNER_CONTAINER_COUNT,
ScmConfigKeys.OZONE_SCM_PIPELINE_OWNER_CONTAINER_COUNT_DEFAULT);
+ this.numPipelinesPerRaftLogDisk = conf
+ .getInt(ScmConfigKeys.OZONE_SCM_PIPELINE_PER_RAFT_LOG_DISK,
+ ScmConfigKeys.OZONE_SCM_PIPELINE_PER_RAFT_LOG_DISK_DEFAULT);
loadExistingContainers();
scmContainerManagerMetrics = SCMContainerManagerMetrics.create();
}
+ private int getOpenContainerCountPerPipeline(Pipeline pipeline) {
+ int totalContainerCountPerDn = numContainerPerVolume *
+ pipelineManager.getNumHealthyVolumes(pipeline);
Review comment:
`pipelineManager.getMaxHealthyVolumeNum(pipeline) / pipelineManager.getMinPipelineLimit(pipeline)` may be more expressive.
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] GlenGeng commented on a change in pull request #1274: HDDS-3810. Add the logic to distribute open containers among the pipelines of a datanode.
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1274:
URL: https://github.com/apache/hadoop-ozone/pull/1274#discussion_r496361400
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/SCMContainerManager.java
##########
@@ -101,12 +103,23 @@ public SCMContainerManager(
this.numContainerPerVolume = conf
.getInt(ScmConfigKeys.OZONE_SCM_PIPELINE_OWNER_CONTAINER_COUNT,
ScmConfigKeys.OZONE_SCM_PIPELINE_OWNER_CONTAINER_COUNT_DEFAULT);
+ this.numPipelinesPerRaftLogDisk = conf
+ .getInt(ScmConfigKeys.OZONE_SCM_PIPELINE_PER_RAFT_LOG_DISK,
+ ScmConfigKeys.OZONE_SCM_PIPELINE_PER_RAFT_LOG_DISK_DEFAULT);
loadExistingContainers();
scmContainerManagerMetrics = SCMContainerManagerMetrics.create();
}
+ private int getOpenContainerCountPerPipeline(Pipeline pipeline) {
+ int totalContainerCountPerDn = numContainerPerVolume *
+ pipelineManager.getNumHealthyVolumes(pipeline);
+ int maxPipelineCountPerDn = pipelineManager.maxPipelineLimit(pipeline);
+ return (int) Math.ceil(
+ ((double) totalContainerCountPerDn / maxPipelineCountPerDn));
+ }
+
Review comment:
might need change pipeline placement policy to make sure that only allocating pipelines on homogeneous machiens.
It one pipeline connects a strong DN and weak DN, its open container number will be `volumeNumOfStrongDN * 3 / pipelineNumOfWeakDN.`.
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] nandakumar131 commented on a change in pull request #1274: HDDS-3810. Add the logic to distribute open containers among the pipelines of a datanode.
Posted by GitBox <gi...@apache.org>.
nandakumar131 commented on a change in pull request #1274:
URL: https://github.com/apache/hadoop-ozone/pull/1274#discussion_r496111822
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeInfo.java
##########
@@ -121,6 +143,19 @@ public int getHealthyVolumeCount() {
}
}
+ /**
+ * Returns count of healthy raft log volumes reported from datanode.
+ * @return count of healthy raft log volumes
+ */
+ public int getRaftLogVolumeCount() {
Review comment:
Rename suggestion: `getMetadataVolumeCount()`
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/SCMContainerManager.java
##########
@@ -101,12 +103,23 @@ public SCMContainerManager(
this.numContainerPerVolume = conf
.getInt(ScmConfigKeys.OZONE_SCM_PIPELINE_OWNER_CONTAINER_COUNT,
ScmConfigKeys.OZONE_SCM_PIPELINE_OWNER_CONTAINER_COUNT_DEFAULT);
+ this.numPipelinesPerRaftLogDisk = conf
+ .getInt(ScmConfigKeys.OZONE_SCM_PIPELINE_PER_RAFT_LOG_DISK,
+ ScmConfigKeys.OZONE_SCM_PIPELINE_PER_RAFT_LOG_DISK_DEFAULT);
loadExistingContainers();
scmContainerManagerMetrics = SCMContainerManagerMetrics.create();
}
+ private int getOpenContainerCountPerPipeline(Pipeline pipeline) {
+ int totalContainerCountPerDn = numContainerPerVolume *
+ pipelineManager.getNumHealthyVolumes(pipeline);
+ int maxPipelineCountPerDn = pipelineManager.maxPipelineLimit(pipeline);
+ return (int) Math.ceil(
+ ((double) totalContainerCountPerDn / maxPipelineCountPerDn));
+ }
+
Review comment:
Will this work in case of heterogeneous datanodes, where one datanode has 1 Raft log disk with 2 data disk and the other datanode has 5 Raft log disk with 10 data disk?
According to the current logic `getOpenContainerCountPerPipeline` will return 10, if `numContainerPerVolume` and `numPipelinesPerRaftLogDisk` are set to 2.
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] elek commented on pull request #1274: HDDS-3810. Add the logic to distribute open containers among the pipelines of a datanode.
Posted by GitBox <gi...@apache.org>.
elek commented on pull request #1274:
URL: https://github.com/apache/hadoop-ozone/pull/1274#issuecomment-679208064
Green build + approval. Will merge this after a new build.
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1274: HDDS-3810. Add the logic to distribute open containers among the pipelines of a datanode.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1274:
URL: https://github.com/apache/hadoop-ozone/pull/1274#discussion_r462883821
##########
File path: hadoop-hdds/common/src/main/resources/ozone-default.xml
##########
@@ -821,6 +821,13 @@
<description>Number of containers per owner per disk in a pipeline.
</description>
</property>
+ <property>
+ <name>ozone.scm.pipeline.per.raft.log.disk</name>
+ <value>3</value>
Review comment:
Shouldn't this be same as `OZONE_SCM_PIPELINE_PER_RAFT_LOG_DISK_DEFAULT`?
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] nandakumar131 merged pull request #1274: HDDS-3810. Add the logic to distribute open containers among the pipelines of a datanode.
Posted by GitBox <gi...@apache.org>.
nandakumar131 merged pull request #1274:
URL: https://github.com/apache/hadoop-ozone/pull/1274
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bshashikant commented on pull request #1274: HDDS-3810. Add the logic to distribute open containers among the pipelines of a datanode.
Posted by GitBox <gi...@apache.org>.
bshashikant commented on pull request #1274:
URL: https://github.com/apache/hadoop-ozone/pull/1274#issuecomment-681846121
This needs some more changes and i will be updating it very soon.
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1274: HDDS-3810. Add the logic to distribute open containers among the pipelines of a datanode.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1274:
URL: https://github.com/apache/hadoop-ozone/pull/1274#discussion_r462876760
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfigKeys.java
##########
@@ -307,6 +307,10 @@
OZONE_SCM_KEY_VALUE_CONTAINER_DELETION_CHOOSING_POLICY =
"ozone.scm.keyvalue.container.deletion-choosing.policy";
+ public static final String OZONE_SCM_PIPELINE_PER_RAFT_LOG_DISK =
+ "ozone.scm.pipeline.per.raft.log.disk";
Review comment:
Should be added to `ozone-default.xml`, too (indicated by failure of `TestOzoneConfigurationFields`).
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] nandakumar131 commented on pull request #1274: HDDS-3810. Add the logic to distribute open containers among the pipelines of a datanode.
Posted by GitBox <gi...@apache.org>.
nandakumar131 commented on pull request #1274:
URL: https://github.com/apache/hadoop-ozone/pull/1274#issuecomment-703405601
+1, 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bshashikant commented on a change in pull request #1274: HDDS-3810. Add the logic to distribute open containers among the pipelines of a datanode.
Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #1274:
URL: https://github.com/apache/hadoop-ozone/pull/1274#discussion_r497311610
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/SCMContainerManager.java
##########
@@ -101,12 +103,23 @@ public SCMContainerManager(
this.numContainerPerVolume = conf
.getInt(ScmConfigKeys.OZONE_SCM_PIPELINE_OWNER_CONTAINER_COUNT,
ScmConfigKeys.OZONE_SCM_PIPELINE_OWNER_CONTAINER_COUNT_DEFAULT);
+ this.numPipelinesPerRaftLogDisk = conf
+ .getInt(ScmConfigKeys.OZONE_SCM_PIPELINE_PER_RAFT_LOG_DISK,
+ ScmConfigKeys.OZONE_SCM_PIPELINE_PER_RAFT_LOG_DISK_DEFAULT);
loadExistingContainers();
scmContainerManagerMetrics = SCMContainerManagerMetrics.create();
}
+ private int getOpenContainerCountPerPipeline(Pipeline pipeline) {
+ int totalContainerCountPerDn = numContainerPerVolume *
+ pipelineManager.getNumHealthyVolumes(pipeline);
+ int maxPipelineCountPerDn = pipelineManager.maxPipelineLimit(pipeline);
+ return (int) Math.ceil(
+ ((double) totalContainerCountPerDn / maxPipelineCountPerDn));
+ }
+
Review comment:
Changed the logic to take the min of the data disks count of the set of dns in a pipeline while determining the open container limit.
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bshashikant commented on pull request #1274: HDDS-3810. Add the logic to distribute open containers among the pipelines of a datanode.
Posted by GitBox <gi...@apache.org>.
bshashikant commented on pull request #1274:
URL: https://github.com/apache/hadoop-ozone/pull/1274#issuecomment-682493126
@nandakumar131 @ChenSammi can you plz 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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] GlenGeng commented on a change in pull request #1274: HDDS-3810. Add the logic to distribute open containers among the pipelines of a datanode.
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1274:
URL: https://github.com/apache/hadoop-ozone/pull/1274#discussion_r496361400
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/SCMContainerManager.java
##########
@@ -101,12 +103,23 @@ public SCMContainerManager(
this.numContainerPerVolume = conf
.getInt(ScmConfigKeys.OZONE_SCM_PIPELINE_OWNER_CONTAINER_COUNT,
ScmConfigKeys.OZONE_SCM_PIPELINE_OWNER_CONTAINER_COUNT_DEFAULT);
+ this.numPipelinesPerRaftLogDisk = conf
+ .getInt(ScmConfigKeys.OZONE_SCM_PIPELINE_PER_RAFT_LOG_DISK,
+ ScmConfigKeys.OZONE_SCM_PIPELINE_PER_RAFT_LOG_DISK_DEFAULT);
loadExistingContainers();
scmContainerManagerMetrics = SCMContainerManagerMetrics.create();
}
+ private int getOpenContainerCountPerPipeline(Pipeline pipeline) {
+ int totalContainerCountPerDn = numContainerPerVolume *
+ pipelineManager.getNumHealthyVolumes(pipeline);
+ int maxPipelineCountPerDn = pipelineManager.maxPipelineLimit(pipeline);
+ return (int) Math.ceil(
+ ((double) totalContainerCountPerDn / maxPipelineCountPerDn));
+ }
+
Review comment:
might need change pipeline placement policy to make sure that only allocating pipelines on homogeneous machiens.
It one pipeline connects a strong DN and weak DN, its open container number will be `volumeNumOfStrongDN * 3 / pipelineNumOfWeakDN.`，pipeline on the same DN may not be able to evenly distribute open containers of that DN.
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bshashikant commented on a change in pull request #1274: HDDS-3810. Add the logic to distribute open containers among the pipelines of a datanode.
Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #1274:
URL: https://github.com/apache/hadoop-ozone/pull/1274#discussion_r496394076
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/SCMContainerManager.java
##########
@@ -101,12 +103,23 @@ public SCMContainerManager(
this.numContainerPerVolume = conf
.getInt(ScmConfigKeys.OZONE_SCM_PIPELINE_OWNER_CONTAINER_COUNT,
ScmConfigKeys.OZONE_SCM_PIPELINE_OWNER_CONTAINER_COUNT_DEFAULT);
+ this.numPipelinesPerRaftLogDisk = conf
+ .getInt(ScmConfigKeys.OZONE_SCM_PIPELINE_PER_RAFT_LOG_DISK,
+ ScmConfigKeys.OZONE_SCM_PIPELINE_PER_RAFT_LOG_DISK_DEFAULT);
loadExistingContainers();
scmContainerManagerMetrics = SCMContainerManagerMetrics.create();
}
+ private int getOpenContainerCountPerPipeline(Pipeline pipeline) {
+ int totalContainerCountPerDn = numContainerPerVolume *
+ pipelineManager.getNumHealthyVolumes(pipeline);
+ int maxPipelineCountPerDn = pipelineManager.maxPipelineLimit(pipeline);
+ return (int) Math.ceil(
+ ((double) totalContainerCountPerDn / maxPipelineCountPerDn));
+ }
+
Review comment:
I agree with @GlenGeng here. The pipeline placement should choose homogeneous datanodes. The choice being made here, have many containers open on a minimal set of pipelines that we can have out of the set of datanodes in the pipleines.
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bshashikant commented on a change in pull request #1274: HDDS-3810. Add the logic to distribute open containers among the pipelines of a datanode.
Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #1274:
URL: https://github.com/apache/hadoop-ozone/pull/1274#discussion_r496394076
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/SCMContainerManager.java
##########
@@ -101,12 +103,23 @@ public SCMContainerManager(
this.numContainerPerVolume = conf
.getInt(ScmConfigKeys.OZONE_SCM_PIPELINE_OWNER_CONTAINER_COUNT,
ScmConfigKeys.OZONE_SCM_PIPELINE_OWNER_CONTAINER_COUNT_DEFAULT);
+ this.numPipelinesPerRaftLogDisk = conf
+ .getInt(ScmConfigKeys.OZONE_SCM_PIPELINE_PER_RAFT_LOG_DISK,
+ ScmConfigKeys.OZONE_SCM_PIPELINE_PER_RAFT_LOG_DISK_DEFAULT);
loadExistingContainers();
scmContainerManagerMetrics = SCMContainerManagerMetrics.create();
}
+ private int getOpenContainerCountPerPipeline(Pipeline pipeline) {
+ int totalContainerCountPerDn = numContainerPerVolume *
+ pipelineManager.getNumHealthyVolumes(pipeline);
+ int maxPipelineCountPerDn = pipelineManager.maxPipelineLimit(pipeline);
+ return (int) Math.ceil(
+ ((double) totalContainerCountPerDn / maxPipelineCountPerDn));
+ }
+
Review comment:
I agree with @GlenGeng here. The pipeline placement should choose homogeneous datanodes. The choice being made here, have many containers open on a minimal set of pipelines that we can have out of the set of datanodes in the pipelines.
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] GlenGeng commented on a change in pull request #1274: HDDS-3810. Add the logic to distribute open containers among the pipelines of a datanode.
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1274:
URL: https://github.com/apache/hadoop-ozone/pull/1274#discussion_r497350686
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/SCMContainerManager.java
##########
@@ -107,6 +107,14 @@ public SCMContainerManager(
scmContainerManagerMetrics = SCMContainerManagerMetrics.create();
}
+ private int getOpenContainerCountPerPipeline(Pipeline pipeline) {
+ int totalContainerCountPerDn = numContainerPerVolume *
Review comment:
name suggestion: totalContainerCountPerDn -> minContainerCountPerDn
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
##########
@@ -130,6 +133,11 @@ public SCMNodeManager(OzoneConfiguration conf,
this.useHostname = conf.getBoolean(
DFSConfigKeysLegacy.DFS_DATANODE_USE_DN_HOSTNAME,
DFSConfigKeysLegacy.DFS_DATANODE_USE_DN_HOSTNAME_DEFAULT);
+ this.numPipelinesPerRaftLogDisk =
Review comment:
member var name suggestion: numPipelinesPerMetadataVolume
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
##########
@@ -511,7 +521,7 @@ private SCMNodeStat getNodeStatInternal(DatanodeDetails datanodeDetails) {
}
/**
- * Returns the max of no healthy volumes reported out of the set
+ * Returns the min of no healthy volumes reported out of the set
Review comment:
method name suggestion: minHealthyVolumeNum
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfigKeys.java
##########
@@ -308,6 +308,10 @@
OZONE_SCM_KEY_VALUE_CONTAINER_DELETION_CHOOSING_POLICY =
"ozone.scm.keyvalue.container.deletion-choosing.policy";
+ public static final String OZONE_SCM_PIPELINE_PER_METADATA_DISK =
Review comment:
name suggestion: OZONE_SCM_PIPELINE_PER_METADATA_VOLUME.
We'd better use concept volume instead of concept disk in SCM, the former one is a logical concept, the latter is physical one, and volume is already widely used in context.
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org