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 2022/12/19 07:21:49 UTC
[GitHub] [ozone] ArafatKhan2198 opened a new pull request, #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
ArafatKhan2198 opened a new pull request, #4107:
URL: https://github.com/apache/ozone/pull/4107
## What changes were proposed in this pull request?
Just like how the file size is calculated across the cluster, I have added a Container Size Count task that will pereodically run after a time interval and store the container size distribution across the cluster in the derby sql database table `ContainerCountBySizeTable`
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7632?filter=-1
## How was this patch tested?
Will be writing Unit Tests for it
--
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] devmadhuu commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1052949796
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerSizeCountTask.java:
##########
@@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.ozone.recon.ReconUtils;
+import org.apache.hadoop.ozone.recon.scm.ReconScmTask;
+import org.apache.hadoop.ozone.recon.spi.StorageContainerServiceProvider;
+import org.hadoop.ozone.recon.schema.tables.daos.ContainerCountBySizeDao;
+import org.hadoop.ozone.recon.schema.tables.daos.ReconTaskStatusDao;
+import org.hadoop.ozone.recon.schema.tables.pojos.ContainerCountBySize;
+import org.jooq.DSLContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.hadoop.ozone.recon.schema.tables.ContainerCountBySizeTable.CONTAINER_COUNT_BY_SIZE;
+
+
+/**
+ * Class that scans the list of containers and keeps track of container sizes
+ * binned into ranges (1KB, 2Kb..,4MB,.., 1TB,..1PB) to the Recon
+ * containerSize DB.
+ */
+public class ContainerSizeCountTask extends ReconScmTask {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ContainerSizeCountTask.class);
+
+ private StorageContainerServiceProvider scmClient;
+ private ContainerManager containerManager;
+ private final long interval;
+ private ContainerCountBySizeDao containerCountBySizeDao;
+ private DSLContext dslContext;
+
+
+ public ContainerSizeCountTask(
+ ContainerManager containerManager,
+ StorageContainerServiceProvider scmClient,
+ ReconTaskStatusDao reconTaskStatusDao,
+ ReconTaskConfig reconTaskConfig,
+ ContainerCountBySizeDao containerCountBySizeDao,
+ DSLContext dslContext) {
+ super(reconTaskStatusDao);
+ this.scmClient = scmClient;
+ this.containerManager = containerManager;
+ this.containerCountBySizeDao = containerCountBySizeDao;
+ this.dslContext = dslContext;
+ interval = reconTaskConfig.getContainerSizeCountTaskInterval().toMillis();
+ }
+
+
+ @Override
+ protected synchronized void run() {
+ try {
+ while (canRun()) {
+ wait(interval);
+
+ final List<ContainerInfo> containers = containerManager.getContainers();
+ Map<ContainerSizeCountKey, Long> containerSizeCountMap =
+ new HashMap<>();
+ for (int i = 0; i < containers.size(); i++) {
+ handlePutKeyEvent(containers.get(i), containerSizeCountMap);
+ }
+ // Truncate table before inserting new rows
+ int execute = dslContext.delete(CONTAINER_COUNT_BY_SIZE).execute();
+ LOG.info("Deleted {} records from {}", execute,
+ CONTAINER_COUNT_BY_SIZE);
+
+ writeCountsToDB(true, containerSizeCountMap);
+
+ LOG.info("Completed a 'reprocess' run of ContainerSizeCountTask.");
+ }
+ } catch (Throwable t) {
+ LOG.error("Exception in Container Size Distribution task Thread.", t);
+ if (t instanceof InterruptedException) {
+ Thread.currentThread().interrupt();
+ }
+ }
+ }
+
+ /**
+ * Populate DB with the counts of container sizes calculated
+ * using the dao.
+ */
+ private void writeCountsToDB(boolean isDbTruncated,
Review Comment:
Do we need this "isDBTruncated" flag if it is always true ?
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerSizeCountTask.java:
##########
@@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.ozone.recon.ReconUtils;
+import org.apache.hadoop.ozone.recon.scm.ReconScmTask;
+import org.apache.hadoop.ozone.recon.spi.StorageContainerServiceProvider;
+import org.hadoop.ozone.recon.schema.tables.daos.ContainerCountBySizeDao;
+import org.hadoop.ozone.recon.schema.tables.daos.ReconTaskStatusDao;
+import org.hadoop.ozone.recon.schema.tables.pojos.ContainerCountBySize;
+import org.jooq.DSLContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.hadoop.ozone.recon.schema.tables.ContainerCountBySizeTable.CONTAINER_COUNT_BY_SIZE;
+
+
+/**
+ * Class that scans the list of containers and keeps track of container sizes
+ * binned into ranges (1KB, 2Kb..,4MB,.., 1TB,..1PB) to the Recon
+ * containerSize DB.
+ */
+public class ContainerSizeCountTask extends ReconScmTask {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ContainerSizeCountTask.class);
+
+ private StorageContainerServiceProvider scmClient;
+ private ContainerManager containerManager;
+ private final long interval;
+ private ContainerCountBySizeDao containerCountBySizeDao;
+ private DSLContext dslContext;
+
+
+ public ContainerSizeCountTask(
+ ContainerManager containerManager,
+ StorageContainerServiceProvider scmClient,
+ ReconTaskStatusDao reconTaskStatusDao,
+ ReconTaskConfig reconTaskConfig,
+ ContainerCountBySizeDao containerCountBySizeDao,
+ DSLContext dslContext) {
+ super(reconTaskStatusDao);
+ this.scmClient = scmClient;
+ this.containerManager = containerManager;
+ this.containerCountBySizeDao = containerCountBySizeDao;
+ this.dslContext = dslContext;
+ interval = reconTaskConfig.getContainerSizeCountTaskInterval().toMillis();
+ }
+
+
+ @Override
+ protected synchronized void run() {
+ try {
+ while (canRun()) {
+ wait(interval);
+
+ final List<ContainerInfo> containers = containerManager.getContainers();
+ Map<ContainerSizeCountKey, Long> containerSizeCountMap =
+ new HashMap<>();
+ for (int i = 0; i < containers.size(); i++) {
+ handlePutKeyEvent(containers.get(i), containerSizeCountMap);
+ }
+ // Truncate table before inserting new rows
+ int execute = dslContext.delete(CONTAINER_COUNT_BY_SIZE).execute();
+ LOG.info("Deleted {} records from {}", execute,
+ CONTAINER_COUNT_BY_SIZE);
+
+ writeCountsToDB(true, containerSizeCountMap);
+
+ LOG.info("Completed a 'reprocess' run of ContainerSizeCountTask.");
+ }
+ } catch (Throwable t) {
+ LOG.error("Exception in Container Size Distribution task Thread.", t);
+ if (t instanceof InterruptedException) {
+ Thread.currentThread().interrupt();
+ }
+ }
+ }
+
+ /**
+ * Populate DB with the counts of container sizes calculated
+ * using the dao.
+ */
+ private void writeCountsToDB(boolean isDbTruncated,
+ Map<ContainerSizeCountKey, Long>
+ containerSizeCountMap) {
+ List<ContainerCountBySize> insertToDb = new ArrayList<>();
+
+ containerSizeCountMap.keySet().forEach((ContainerSizeCountKey key) -> {
+ ContainerCountBySize newRecord = new ContainerCountBySize();
+ newRecord.setContainerSize(key.containerSizeUpperBound);
+ newRecord.setCount(containerSizeCountMap.get(key));
+ if (!isDbTruncated) {
+ insertToDb.add(newRecord);
+ }
+ });
+ containerCountBySizeDao.insert(insertToDb);
+ }
+
+ @Override
+ public String getTaskName() {
+ return "ContainerSizeCountTask";
+ }
+
+ /**
+ * Calculate and update the count of containers being tracked by
+ * containerSizeCountMap.
+ * Used by reprocess() and process().
+ *
+ * @param containerInfo OmKey being updated for count
+ */
+ private void handlePutKeyEvent(ContainerInfo containerInfo,
Review Comment:
Try to give better name, as there is no event we are capturing with SCM DB sync, here we are taking data from Recon container cache.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ReconTaskConfig.java:
##########
@@ -66,4 +66,22 @@ public void setMissingContainerTaskInterval(Duration interval) {
this.missingContainerTaskInterval = interval.toMillis();
}
+ @Config(key = "containercounttask.interval",
+ type = ConfigType.TIME,
+ defaultValue = "300s",
+ tags = { ConfigTag.RECON, ConfigTag.OZONE },
+ description = "The time interval of the periodic check for " +
+ "container size distribution in the cluster as reported "
+ )
+ private long containerSizeCountTaskInterval =
+ Duration.ofMinutes(1).toMillis();
Review Comment:
Why setting 1 minute here when we are defining the config value default as 300s ?
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerSizeCountTask.java:
##########
@@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.ozone.recon.ReconUtils;
+import org.apache.hadoop.ozone.recon.scm.ReconScmTask;
+import org.apache.hadoop.ozone.recon.spi.StorageContainerServiceProvider;
+import org.hadoop.ozone.recon.schema.tables.daos.ContainerCountBySizeDao;
+import org.hadoop.ozone.recon.schema.tables.daos.ReconTaskStatusDao;
+import org.hadoop.ozone.recon.schema.tables.pojos.ContainerCountBySize;
+import org.jooq.DSLContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.hadoop.ozone.recon.schema.tables.ContainerCountBySizeTable.CONTAINER_COUNT_BY_SIZE;
+
+
+/**
+ * Class that scans the list of containers and keeps track of container sizes
+ * binned into ranges (1KB, 2Kb..,4MB,.., 1TB,..1PB) to the Recon
+ * containerSize DB.
+ */
+public class ContainerSizeCountTask extends ReconScmTask {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ContainerSizeCountTask.class);
+
+ private StorageContainerServiceProvider scmClient;
+ private ContainerManager containerManager;
+ private final long interval;
+ private ContainerCountBySizeDao containerCountBySizeDao;
+ private DSLContext dslContext;
+
+
+ public ContainerSizeCountTask(
+ ContainerManager containerManager,
+ StorageContainerServiceProvider scmClient,
+ ReconTaskStatusDao reconTaskStatusDao,
+ ReconTaskConfig reconTaskConfig,
+ ContainerCountBySizeDao containerCountBySizeDao,
+ DSLContext dslContext) {
+ super(reconTaskStatusDao);
+ this.scmClient = scmClient;
+ this.containerManager = containerManager;
+ this.containerCountBySizeDao = containerCountBySizeDao;
+ this.dslContext = dslContext;
+ interval = reconTaskConfig.getContainerSizeCountTaskInterval().toMillis();
+ }
+
+
+ @Override
+ protected synchronized void run() {
+ try {
+ while (canRun()) {
+ wait(interval);
+
+ final List<ContainerInfo> containers = containerManager.getContainers();
+ Map<ContainerSizeCountKey, Long> containerSizeCountMap =
+ new HashMap<>();
+ for (int i = 0; i < containers.size(); i++) {
+ handlePutKeyEvent(containers.get(i), containerSizeCountMap);
+ }
+ // Truncate table before inserting new rows
+ int execute = dslContext.delete(CONTAINER_COUNT_BY_SIZE).execute();
+ LOG.info("Deleted {} records from {}", execute,
+ CONTAINER_COUNT_BY_SIZE);
+
+ writeCountsToDB(true, containerSizeCountMap);
+
+ LOG.info("Completed a 'reprocess' run of ContainerSizeCountTask.");
+ }
+ } catch (Throwable t) {
+ LOG.error("Exception in Container Size Distribution task Thread.", t);
+ if (t instanceof InterruptedException) {
+ Thread.currentThread().interrupt();
+ }
+ }
+ }
+
+ /**
+ * Populate DB with the counts of container sizes calculated
+ * using the dao.
+ */
+ private void writeCountsToDB(boolean isDbTruncated,
+ Map<ContainerSizeCountKey, Long>
+ containerSizeCountMap) {
+ List<ContainerCountBySize> insertToDb = new ArrayList<>();
+
+ containerSizeCountMap.keySet().forEach((ContainerSizeCountKey key) -> {
+ ContainerCountBySize newRecord = new ContainerCountBySize();
+ newRecord.setContainerSize(key.containerSizeUpperBound);
+ newRecord.setCount(containerSizeCountMap.get(key));
+ if (!isDbTruncated) {
+ insertToDb.add(newRecord);
+ }
+ });
+ containerCountBySizeDao.insert(insertToDb);
+ }
+
+ @Override
+ public String getTaskName() {
+ return "ContainerSizeCountTask";
+ }
+
+ /**
+ * Calculate and update the count of containers being tracked by
+ * containerSizeCountMap.
+ * Used by reprocess() and process().
Review Comment:
Not sure if this comment is true. Don't see any reprocess or process method.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerSizeCountTask.java:
##########
@@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.ozone.recon.ReconUtils;
+import org.apache.hadoop.ozone.recon.scm.ReconScmTask;
+import org.apache.hadoop.ozone.recon.spi.StorageContainerServiceProvider;
+import org.hadoop.ozone.recon.schema.tables.daos.ContainerCountBySizeDao;
+import org.hadoop.ozone.recon.schema.tables.daos.ReconTaskStatusDao;
+import org.hadoop.ozone.recon.schema.tables.pojos.ContainerCountBySize;
+import org.jooq.DSLContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.hadoop.ozone.recon.schema.tables.ContainerCountBySizeTable.CONTAINER_COUNT_BY_SIZE;
+
+
+/**
+ * Class that scans the list of containers and keeps track of container sizes
+ * binned into ranges (1KB, 2Kb..,4MB,.., 1TB,..1PB) to the Recon
+ * containerSize DB.
+ */
+public class ContainerSizeCountTask extends ReconScmTask {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ContainerSizeCountTask.class);
+
+ private StorageContainerServiceProvider scmClient;
+ private ContainerManager containerManager;
+ private final long interval;
+ private ContainerCountBySizeDao containerCountBySizeDao;
+ private DSLContext dslContext;
+
+
+ public ContainerSizeCountTask(
+ ContainerManager containerManager,
+ StorageContainerServiceProvider scmClient,
+ ReconTaskStatusDao reconTaskStatusDao,
+ ReconTaskConfig reconTaskConfig,
+ ContainerCountBySizeDao containerCountBySizeDao,
+ DSLContext dslContext) {
+ super(reconTaskStatusDao);
+ this.scmClient = scmClient;
+ this.containerManager = containerManager;
+ this.containerCountBySizeDao = containerCountBySizeDao;
+ this.dslContext = dslContext;
+ interval = reconTaskConfig.getContainerSizeCountTaskInterval().toMillis();
+ }
+
+
+ @Override
+ protected synchronized void run() {
+ try {
+ while (canRun()) {
+ wait(interval);
+
+ final List<ContainerInfo> containers = containerManager.getContainers();
+ Map<ContainerSizeCountKey, Long> containerSizeCountMap =
+ new HashMap<>();
+ for (int i = 0; i < containers.size(); i++) {
+ handlePutKeyEvent(containers.get(i), containerSizeCountMap);
+ }
+ // Truncate table before inserting new rows
+ int execute = dslContext.delete(CONTAINER_COUNT_BY_SIZE).execute();
+ LOG.info("Deleted {} records from {}", execute,
+ CONTAINER_COUNT_BY_SIZE);
+
+ writeCountsToDB(true, containerSizeCountMap);
+
+ LOG.info("Completed a 'reprocess' run of ContainerSizeCountTask.");
Review Comment:
Correct this comment.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ReconTaskConfig.java:
##########
@@ -66,4 +66,22 @@ public void setMissingContainerTaskInterval(Duration interval) {
this.missingContainerTaskInterval = interval.toMillis();
}
+ @Config(key = "containercounttask.interval",
+ type = ConfigType.TIME,
+ defaultValue = "300s",
+ tags = { ConfigTag.RECON, ConfigTag.OZONE },
+ description = "The time interval of the periodic check for " +
+ "container size distribution in the cluster as reported "
+ )
+ private long containerSizeCountTaskInterval =
+ Duration.ofMinutes(1).toMillis();
+
+ public Duration getContainerSizeCountTaskInterval() {
+ return Duration.ofMillis(missingContainerTaskInterval);
+ }
+
+ public void setContainerSizeCountTaskInterval(Duration interval) {
+ this.missingContainerTaskInterval = interval.toMillis();
Review Comment:
Why we are getting and setting missingContainerTaskInterval ? This is different task.
--
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] ArafatKhan2198 commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by GitBox <gi...@apache.org>.
ArafatKhan2198 commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1070858951
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconStaleNodeHandler.java:
##########
@@ -50,6 +55,7 @@ public void onMessage(final DatanodeDetails datanodeDetails,
super.onMessage(datanodeDetails, publisher);
try {
pipelineSyncTask.triggerPipelineSyncTask();
+ containerSizeCountTask.process(containerManager.getContainers());
Review Comment:
The main function of the stale node handler is to ensure that the data stored in the pipelines associated with the stale node is not accessible anymore and also log the information about the stale node.
**Hence, since neither containers or their replicas are removed, the container size counts will not be affected. Therefore, we would not have to trigger the ContainerSizeCountTask.**
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconIncrementalContainerReportHandler.java:
##########
@@ -99,5 +103,6 @@ public void onMessage(final IncrementalContainerReportFromDatanode report,
}
}
containerManager.notifyContainerReportProcessing(false, success);
+ containerSizeCountTask.process(containerManager.getContainers());
Review Comment:
Since If the container does not exist, it creates a new container and adds it to the ContainerManager hence we will have to trigger the ContainerSizeCountTask.
--
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] ArafatKhan2198 commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by GitBox <gi...@apache.org>.
ArafatKhan2198 commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1070858850
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconDeadNodeHandler.java:
##########
@@ -79,6 +85,7 @@ public void onMessage(final DatanodeDetails datanodeDetails,
}
containerHealthTask.triggerContainerHealthCheck();
pipelineSyncTask.triggerPipelineSyncTask();
+ containerSizeCountTask.process(containerManager.getContainers());
Review Comment:
For DeadDatanodeHandler we will be calling **destroyPipelines, closeContainers and removeContainerReplicas methods.** Hence since the container replicas are removed, the container size counts will also be affected. Therefore, we need to trigger the ContainerSizeCountTask.
--
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] ArafatKhan2198 commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by GitBox <gi...@apache.org>.
ArafatKhan2198 commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1070859340
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconIncrementalContainerReportHandler.java:
##########
@@ -99,5 +103,6 @@ public void onMessage(final IncrementalContainerReportFromDatanode report,
}
}
containerManager.notifyContainerReportProcessing(false, success);
+ containerSizeCountTask.process(containerManager.getContainers());
Review Comment:
I have written in detail about how each of the three handlers work, in this google doc :-
https://docs.google.com/document/d/1Wz-zeSoSPfdaVyw43GwAyvFNgt0djo5w44j8OjrxJ1I/edit#
--
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] ArafatKhan2198 commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1168239869
##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/persistence/TestUtilizationSchemaDefinition.java:
##########
@@ -103,6 +104,21 @@ public void testReconSchemaCreated() throws Exception {
"Unexpected number of columns");
assertEquals(expectedPairsFileCount, actualPairsFileCount,
"Columns Do not Match ");
+
+
+ ResultSet resultSetContainerCount = metaData.getColumns(null, null,
+ CONTAINER_COUNT_BY_SIZE_TABLE_NAME, null);
+
Review Comment:
Good find @dombizita Ill make the changes !!
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.
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] dombizita commented on pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on PR #4107:
URL: https://github.com/apache/ozone/pull/4107#issuecomment-1521903006
> So after a few discussions with @nandakumar131 The conversation was about adding a trigger to the `IncrementalContainerReportHandler` in order to trigger the `ContainerCountTask` whenever the handler is executed. It was determined that the `ICRReportHandler` will be triggered frequently in a busy cluster, and adding a trigger would put additional load on the cluster. Therefore, the final decision was not to add the trigger to the handler.
>
> @devmadhuu @dombizita please take a final look at this !!
during the discussion did you talk about [ReconDeadNodeHandler.java](https://github.com/apache/ozone/pull/4107/files#diff-cf88738ad7edb0171749525ea362b6172a30c36e206e487d0ed916e226c8927d)? do you think it is good to trigger the task beside the periodic runs, is it helping that much in the understanding of container sizes? won't it cause frequent calls in some cases?
beside this I am fine with your changes, I added a few small comments to remove unnecessary code changes in some classes. I'd like to have @devmadhuu to review it as well as I am not sure if all of his previous comments have been addressed.
--
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] ArafatKhan2198 commented on pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on PR #4107:
URL: https://github.com/apache/ozone/pull/4107#issuecomment-1510801419
https://github.com/apache/ozone/pull/4107#discussion_r1166750547
So the DeadNodeHandler calls the `removeContainerReplicas` method, which is responsible for removing the container replicas associated with the dead datanode, unless it is in maintenance. Hence, since the container replicas are removed, the container size counts will also be affected. Therefore, we need to trigger the ContainerSizeCountTask, and hence the ContainerSizeCountTask is possible to run more times in each defined interval.
--
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] devmadhuu commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1067742213
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/UtilizationEndpoint.java:
##########
@@ -49,21 +55,24 @@ public class UtilizationEndpoint {
private FileCountBySizeDao fileCountBySizeDao;
private UtilizationSchemaDefinition utilizationSchemaDefinition;
+ private ContainerCountBySizeDao containerCountBySizeDao;
@Inject
public UtilizationEndpoint(FileCountBySizeDao fileCountBySizeDao,
+ ContainerCountBySizeDao containerCountBySizeDao,
UtilizationSchemaDefinition
utilizationSchemaDefinition) {
this.utilizationSchemaDefinition = utilizationSchemaDefinition;
this.fileCountBySizeDao = fileCountBySizeDao;
+ this.containerCountBySizeDao = containerCountBySizeDao;
}
/**
* Return the file counts from Recon DB.
* @return {@link Response}
*/
@GET
- @Path("/fileCount")
+ @Path("/pop")
Review Comment:
If this change is impacting taking care on UI, whether we have created separate task to handle UI changes ? Pls mention and make sure that PR for this change and PR for UI should be merged together.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconStaleNodeHandler.java:
##########
@@ -50,6 +55,7 @@ public void onMessage(final DatanodeDetails datanodeDetails,
super.onMessage(datanodeDetails, publisher);
try {
pipelineSyncTask.triggerPipelineSyncTask();
+ containerSizeCountTask.process(containerManager.getContainers());
Review Comment:
Have we verified if a node becomes stale, how container size will be impacted. Container may get closed on that DN, but some other replica for that container may be present in other DN in cluster. Pls check if we really need to call our task for StaleNode handler.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconDeadNodeHandler.java:
##########
@@ -79,6 +85,7 @@ public void onMessage(final DatanodeDetails datanodeDetails,
}
containerHealthTask.triggerContainerHealthCheck();
pipelineSyncTask.triggerPipelineSyncTask();
+ containerSizeCountTask.process(containerManager.getContainers());
Review Comment:
Have we verified if a node becomes dead, how container size will be impacted. Container may get closed on that DN, but some other replica for that container may be present in other DN in cluster. Pls check if we really need to call our task for DeadNode handler.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconIncrementalContainerReportHandler.java:
##########
@@ -99,5 +103,6 @@ public void onMessage(final IncrementalContainerReportFromDatanode report,
}
}
containerManager.notifyContainerReportProcessing(false, success);
+ containerSizeCountTask.process(containerManager.getContainers());
Review Comment:
As we discussed, ICR handler get called for every DN, so we need to first check if our task map already contains the container, if yes, then it means , our task will not be impacted just for state as our task maintains count based on size. If container not present in our. map, then only we should call our task. Pls check how complex and feasible this to implement. When ICR handler gets called for deletion of containers, we should verify first after whether containerManager is having the container or not, if not then our task can be called to handle for deletion of container.
Please first write down all use case scenarios related to ICR why and when it is needed. Same for Stale and Dead Node handler.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconDeadNodeHandler.java:
##########
@@ -44,17 +45,22 @@ public class ReconDeadNodeHandler extends DeadNodeHandler {
private StorageContainerServiceProvider scmClient;
private ContainerHealthTask containerHealthTask;
private PipelineSyncTask pipelineSyncTask;
+ private ContainerSizeCountTask containerSizeCountTask;
+ private ContainerManager containerManager;
public ReconDeadNodeHandler(NodeManager nodeManager,
PipelineManager pipelineManager,
ContainerManager containerManager,
StorageContainerServiceProvider scmClient,
ContainerHealthTask containerHealthTask,
- PipelineSyncTask pipelineSyncTask) {
+ PipelineSyncTask pipelineSyncTask,
+ ContainerSizeCountTask containerSizeCountTask) {
super(nodeManager, pipelineManager, containerManager);
this.scmClient = scmClient;
+ this.containerManager = containerManager;
Review Comment:
We should not pass containerManager here. It should be available in task itself already.
--
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] jojochuang commented on pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #4107:
URL: https://github.com/apache/ozone/pull/4107#issuecomment-1428266444
Error: Failures:
Error: TestContainerSizeCountTask.testProcess:99 expected: <2> but was: <1>
this test failure is most likely related to this PR. @ArafatKhan2198 please double check
--
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] ArafatKhan2198 commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1168259143
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerSizeCountTask.java:
##########
@@ -0,0 +1,367 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.ozone.recon.ReconUtils;
+import org.apache.hadoop.ozone.recon.scm.ReconScmTask;
+import org.apache.hadoop.ozone.recon.spi.StorageContainerServiceProvider;
+import org.hadoop.ozone.recon.schema.UtilizationSchemaDefinition;
+import org.hadoop.ozone.recon.schema.tables.daos.ContainerCountBySizeDao;
+import org.hadoop.ozone.recon.schema.tables.daos.ReconTaskStatusDao;
+import org.hadoop.ozone.recon.schema.tables.pojos.ContainerCountBySize;
+import org.jooq.DSLContext;
+import org.jooq.Record1;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+import static org.hadoop.ozone.recon.schema.tables.ContainerCountBySizeTable.CONTAINER_COUNT_BY_SIZE;
+
+
+/**
+ * Class that scans the list of containers and keeps track of container sizes
+ * binned into ranges (1KB, 2Kb…,4MB,…1GB,…5GB…) to the Recon
+ * containerSize DB.
+ */
+public class ContainerSizeCountTask extends ReconScmTask {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ContainerSizeCountTask.class);
+
+ private StorageContainerServiceProvider scmClient;
+ private ContainerManager containerManager;
+ private final long interval;
+ private ContainerCountBySizeDao containerCountBySizeDao;
+ private DSLContext dslContext;
+ private HashMap<ContainerID, Long> processedContainers = new HashMap<>();
+ private ReadWriteLock lock = new ReentrantReadWriteLock(true);
+
+ public ContainerSizeCountTask(
+ ContainerManager containerManager,
+ StorageContainerServiceProvider scmClient,
+ ReconTaskStatusDao reconTaskStatusDao,
+ ReconTaskConfig reconTaskConfig,
+ ContainerCountBySizeDao containerCountBySizeDao,
+ UtilizationSchemaDefinition utilizationSchemaDefinition) {
+ super(reconTaskStatusDao);
+ this.scmClient = scmClient;
+ this.containerManager = containerManager;
+ this.containerCountBySizeDao = containerCountBySizeDao;
+ this.dslContext = utilizationSchemaDefinition.getDSLContext();
+ interval = reconTaskConfig.getContainerSizeCountTaskInterval().toMillis();
+ }
+
+
+ /**
+ * The run() method is the main loop of the ContainerSizeCountTask class.
+ * It periodically retrieves a list of containers from the containerManager,
+ * and then calls either to reprocess() or process() method depending on
Review Comment:
Sure will change!!
--
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] ArafatKhan2198 commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by GitBox <gi...@apache.org>.
ArafatKhan2198 commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1067786656
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/UtilizationEndpoint.java:
##########
@@ -49,21 +55,24 @@ public class UtilizationEndpoint {
private FileCountBySizeDao fileCountBySizeDao;
private UtilizationSchemaDefinition utilizationSchemaDefinition;
+ private ContainerCountBySizeDao containerCountBySizeDao;
@Inject
public UtilizationEndpoint(FileCountBySizeDao fileCountBySizeDao,
+ ContainerCountBySizeDao containerCountBySizeDao,
UtilizationSchemaDefinition
utilizationSchemaDefinition) {
this.utilizationSchemaDefinition = utilizationSchemaDefinition;
this.fileCountBySizeDao = fileCountBySizeDao;
+ this.containerCountBySizeDao = containerCountBySizeDao;
}
/**
* Return the file counts from Recon DB.
* @return {@link Response}
*/
@GET
- @Path("/fileCount")
+ @Path("/pop")
Review Comment:
This was by mistake! thanks for pointing it out !!
--
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] ArafatKhan2198 commented on pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on PR #4107:
URL: https://github.com/apache/ozone/pull/4107#issuecomment-1521281291
So after a few discussions with @nandakumar131
The conversation was about adding a trigger to the `IncrementalContainerReportHandler` in order to trigger the `ContainerCountTask` whenever the handler is executed. It was determined that the `ICRReportHandler` will be triggered frequently in a busy cluster, and adding a trigger would put additional load on the cluster. Therefore, the final decision was not to add the trigger to the handler.
@devmadhuu @dombizita please take a final look at 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.
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] DaveTeng0 commented on pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on PR #4107:
URL: https://github.com/apache/ozone/pull/4107#issuecomment-1418671731
Hey @ArafatKhan2198 ~ there were some CI tests failures. Please help take a look when you have some 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.
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] ArafatKhan2198 commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by GitBox <gi...@apache.org>.
ArafatKhan2198 commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1066711858
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerSizeCountTask.java:
##########
@@ -0,0 +1,399 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.ozone.recon.ReconUtils;
+import org.apache.hadoop.ozone.recon.scm.ReconScmTask;
+import org.apache.hadoop.ozone.recon.spi.StorageContainerServiceProvider;
+import org.hadoop.ozone.recon.schema.UtilizationSchemaDefinition;
+import org.hadoop.ozone.recon.schema.tables.daos.ContainerCountBySizeDao;
+import org.hadoop.ozone.recon.schema.tables.daos.ReconTaskStatusDao;
+import org.hadoop.ozone.recon.schema.tables.pojos.ContainerCountBySize;
+import org.jooq.DSLContext;
+import org.jooq.Record1;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.hadoop.ozone.recon.schema.tables.ContainerCountBySizeTable.CONTAINER_COUNT_BY_SIZE;
+
+
+/**
+ * Class that scans the list of containers and keeps track of container sizes
+ * binned into ranges (1KB, 2Kb…,4MB,…1GB,…5GB…) to the Recon
+ * containerSize DB.
+ */
+public class ContainerSizeCountTask extends ReconScmTask {
Review Comment:
Thats a good idea! I'll make the required changes !!
--
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] dombizita commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1176606837
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconIncrementalContainerReportHandler.java:
##########
@@ -46,7 +46,7 @@
ReconIncrementalContainerReportHandler.class);
public ReconIncrementalContainerReportHandler(NodeManager nodeManager,
- ContainerManager containerManager, SCMContext scmContext) {
+ ContainerManager containerManager, SCMContext scmContext) {
Review Comment:
I think we should remove this unnecessary change in the class
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconStorageContainerManagerFacade.java:
##########
@@ -165,9 +174,7 @@ public ReconStorageContainerManagerFacade(OzoneConfiguration conf,
this.eventQueue = new EventQueue();
eventQueue.setSilent(true);
this.scmContext = new SCMContext.Builder()
- .setIsPreCheckComplete(true)
- .setSCM(this)
- .build();
+ .setIsPreCheckComplete(true).setSCM(this).build();
Review Comment:
I think we should remove this unnecessary change in the class (also some others in this class as well)
--
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] devmadhuu commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1157005567
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerSizeCountTask.java:
##########
@@ -0,0 +1,371 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.ozone.recon.ReconUtils;
+import org.apache.hadoop.ozone.recon.scm.ReconScmTask;
+import org.apache.hadoop.ozone.recon.spi.StorageContainerServiceProvider;
+import org.hadoop.ozone.recon.schema.UtilizationSchemaDefinition;
+import org.hadoop.ozone.recon.schema.tables.daos.ContainerCountBySizeDao;
+import org.hadoop.ozone.recon.schema.tables.daos.ReconTaskStatusDao;
+import org.hadoop.ozone.recon.schema.tables.pojos.ContainerCountBySize;
+import org.jooq.DSLContext;
+import org.jooq.Record1;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+import static org.hadoop.ozone.recon.schema.tables.ContainerCountBySizeTable.CONTAINER_COUNT_BY_SIZE;
+
+
+/**
+ * Class that scans the list of containers and keeps track of container sizes
+ * binned into ranges (1KB, 2Kb…,4MB,…1GB,…5GB…) to the Recon
+ * containerSize DB.
+ */
+public class ContainerSizeCountTask extends ReconScmTask {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ContainerSizeCountTask.class);
+
+ private StorageContainerServiceProvider scmClient;
+ private ContainerManager containerManager;
+ private final long interval;
+ private ContainerCountBySizeDao containerCountBySizeDao;
+ private DSLContext dslContext;
+ private HashMap<ContainerID, Long> processedContainers = new HashMap<>();
+ private ReadWriteLock lock = new ReentrantReadWriteLock(true);
+
+ public ContainerSizeCountTask(
+ ContainerManager containerManager,
+ StorageContainerServiceProvider scmClient,
+ ReconTaskStatusDao reconTaskStatusDao,
+ ReconTaskConfig reconTaskConfig,
+ ContainerCountBySizeDao containerCountBySizeDao,
+ UtilizationSchemaDefinition utilizationSchemaDefinition) {
+ super(reconTaskStatusDao);
+ this.scmClient = scmClient;
+ this.containerManager = containerManager;
+ this.containerCountBySizeDao = containerCountBySizeDao;
+ this.dslContext = utilizationSchemaDefinition.getDSLContext();
+ interval = reconTaskConfig.getContainerSizeCountTaskInterval().toMillis();
+ }
+
+
+ /**
+ * The run() method is the main loop of the ContainerSizeCountTask class.
+ * It periodically retrieves a list of containers from the containerManager,
+ * and then calls either to reprocess() or process() method depending on
+ * whether the processedContainers map is empty or not.
+ */
+ @Override
+ protected synchronized void run() {
+ try {
+ while (canRun()) {
+ wait(interval);
+ long startTime, endTime, duration, durationMilliseconds;
+ final List<ContainerInfo> containers = containerManager.getContainers();
+ if (processedContainers.isEmpty()) {
+ try {
+ int execute =
+ dslContext.truncate(CONTAINER_COUNT_BY_SIZE).execute();
+ LOG.info("Deleted {} records from {}", execute,
+ CONTAINER_COUNT_BY_SIZE);
+ } catch (Exception e) {
+ LOG.error("An error occurred while truncating the table {}: {}",
+ CONTAINER_COUNT_BY_SIZE, e.getMessage(), e);
+ return;
+ }
+ }
+ startTime = System.nanoTime();
+ process(containers);
+ endTime = System.nanoTime();
+ duration = endTime - startTime;
+ durationMilliseconds = duration / 1_000_000;
+ LOG.info("Elapsed Time in milli seconds for Process() execution: ",
Review Comment:
Add this to metrics.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerSizeCountTask.java:
##########
@@ -0,0 +1,371 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.ozone.recon.ReconUtils;
+import org.apache.hadoop.ozone.recon.scm.ReconScmTask;
+import org.apache.hadoop.ozone.recon.spi.StorageContainerServiceProvider;
+import org.hadoop.ozone.recon.schema.UtilizationSchemaDefinition;
+import org.hadoop.ozone.recon.schema.tables.daos.ContainerCountBySizeDao;
+import org.hadoop.ozone.recon.schema.tables.daos.ReconTaskStatusDao;
+import org.hadoop.ozone.recon.schema.tables.pojos.ContainerCountBySize;
+import org.jooq.DSLContext;
+import org.jooq.Record1;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+import static org.hadoop.ozone.recon.schema.tables.ContainerCountBySizeTable.CONTAINER_COUNT_BY_SIZE;
+
+
+/**
+ * Class that scans the list of containers and keeps track of container sizes
+ * binned into ranges (1KB, 2Kb…,4MB,…1GB,…5GB…) to the Recon
+ * containerSize DB.
+ */
+public class ContainerSizeCountTask extends ReconScmTask {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ContainerSizeCountTask.class);
+
+ private StorageContainerServiceProvider scmClient;
+ private ContainerManager containerManager;
+ private final long interval;
+ private ContainerCountBySizeDao containerCountBySizeDao;
+ private DSLContext dslContext;
+ private HashMap<ContainerID, Long> processedContainers = new HashMap<>();
+ private ReadWriteLock lock = new ReentrantReadWriteLock(true);
+
+ public ContainerSizeCountTask(
+ ContainerManager containerManager,
+ StorageContainerServiceProvider scmClient,
+ ReconTaskStatusDao reconTaskStatusDao,
+ ReconTaskConfig reconTaskConfig,
+ ContainerCountBySizeDao containerCountBySizeDao,
+ UtilizationSchemaDefinition utilizationSchemaDefinition) {
+ super(reconTaskStatusDao);
+ this.scmClient = scmClient;
+ this.containerManager = containerManager;
+ this.containerCountBySizeDao = containerCountBySizeDao;
+ this.dslContext = utilizationSchemaDefinition.getDSLContext();
+ interval = reconTaskConfig.getContainerSizeCountTaskInterval().toMillis();
+ }
+
+
+ /**
+ * The run() method is the main loop of the ContainerSizeCountTask class.
+ * It periodically retrieves a list of containers from the containerManager,
+ * and then calls either to reprocess() or process() method depending on
+ * whether the processedContainers map is empty or not.
+ */
+ @Override
+ protected synchronized void run() {
+ try {
+ while (canRun()) {
+ wait(interval);
+ long startTime, endTime, duration, durationMilliseconds;
+ final List<ContainerInfo> containers = containerManager.getContainers();
+ if (processedContainers.isEmpty()) {
+ try {
+ int execute =
+ dslContext.truncate(CONTAINER_COUNT_BY_SIZE).execute();
+ LOG.info("Deleted {} records from {}", execute,
+ CONTAINER_COUNT_BY_SIZE);
+ } catch (Exception e) {
+ LOG.error("An error occurred while truncating the table {}: {}",
+ CONTAINER_COUNT_BY_SIZE, e.getMessage(), e);
+ return;
+ }
+ }
+ startTime = System.nanoTime();
+ process(containers);
+ endTime = System.nanoTime();
+ duration = endTime - startTime;
+ durationMilliseconds = duration / 1_000_000;
+ LOG.info("Elapsed Time in milli seconds for Process() execution: ",
+ (durationMilliseconds));
+ }
+ } catch (Throwable t) {
+ LOG.error("Exception in Container Size Distribution task Thread.", t);
+ if (t instanceof InterruptedException) {
+ Thread.currentThread().interrupt();
+ }
+ }
+ }
+
+
+ /**
+ * The process() function is responsible for updating the counts of
+ * containers being tracked in a containerSizeCountMap based on the
+ * ContainerInfo objects in the list containers.It then iterates through
+ * the list of containers and does the following for each container:
+ *
+ * 1) If the container is not present in processedContainers,
+ * it is a new container, so it is added to the processedContainers map
+ * and the count for its size in the containerSizeCountMap is incremented
+ * by 1 using the handlePutKeyEvent() function.
+ * 2) If the container is present in processedContainers but its size has
Review Comment:
Correct the description.
##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/scm/TestReconIncrementalContainerReportHandler.java:
##########
@@ -133,6 +137,8 @@ public void testProcessICRStateMismatch()
containerWithPipeline.getPipeline().getFirstNode();
NodeManager nodeManagerMock = mock(NodeManager.class);
when(nodeManagerMock.getNodeByUuid(any())).thenReturn(datanodeDetails);
+ ContainerSizeCountTask containerSizeCountTask =
+ mock(ContainerSizeCountTask.class);
Review Comment:
Pls see if you can how easy to add assertion for change in size count
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerSizeCountTask.java:
##########
@@ -0,0 +1,371 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.ozone.recon.ReconUtils;
+import org.apache.hadoop.ozone.recon.scm.ReconScmTask;
+import org.apache.hadoop.ozone.recon.spi.StorageContainerServiceProvider;
+import org.hadoop.ozone.recon.schema.UtilizationSchemaDefinition;
+import org.hadoop.ozone.recon.schema.tables.daos.ContainerCountBySizeDao;
+import org.hadoop.ozone.recon.schema.tables.daos.ReconTaskStatusDao;
+import org.hadoop.ozone.recon.schema.tables.pojos.ContainerCountBySize;
+import org.jooq.DSLContext;
+import org.jooq.Record1;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+import static org.hadoop.ozone.recon.schema.tables.ContainerCountBySizeTable.CONTAINER_COUNT_BY_SIZE;
+
+
+/**
+ * Class that scans the list of containers and keeps track of container sizes
+ * binned into ranges (1KB, 2Kb…,4MB,…1GB,…5GB…) to the Recon
+ * containerSize DB.
+ */
+public class ContainerSizeCountTask extends ReconScmTask {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ContainerSizeCountTask.class);
+
+ private StorageContainerServiceProvider scmClient;
+ private ContainerManager containerManager;
+ private final long interval;
+ private ContainerCountBySizeDao containerCountBySizeDao;
+ private DSLContext dslContext;
+ private HashMap<ContainerID, Long> processedContainers = new HashMap<>();
+ private ReadWriteLock lock = new ReentrantReadWriteLock(true);
+
+ public ContainerSizeCountTask(
+ ContainerManager containerManager,
+ StorageContainerServiceProvider scmClient,
+ ReconTaskStatusDao reconTaskStatusDao,
+ ReconTaskConfig reconTaskConfig,
+ ContainerCountBySizeDao containerCountBySizeDao,
+ UtilizationSchemaDefinition utilizationSchemaDefinition) {
+ super(reconTaskStatusDao);
+ this.scmClient = scmClient;
+ this.containerManager = containerManager;
+ this.containerCountBySizeDao = containerCountBySizeDao;
+ this.dslContext = utilizationSchemaDefinition.getDSLContext();
+ interval = reconTaskConfig.getContainerSizeCountTaskInterval().toMillis();
+ }
+
+
+ /**
+ * The run() method is the main loop of the ContainerSizeCountTask class.
+ * It periodically retrieves a list of containers from the containerManager,
+ * and then calls either to reprocess() or process() method depending on
+ * whether the processedContainers map is empty or not.
+ */
+ @Override
+ protected synchronized void run() {
+ try {
+ while (canRun()) {
+ wait(interval);
+ long startTime, endTime, duration, durationMilliseconds;
+ final List<ContainerInfo> containers = containerManager.getContainers();
+ if (processedContainers.isEmpty()) {
+ try {
+ int execute =
+ dslContext.truncate(CONTAINER_COUNT_BY_SIZE).execute();
+ LOG.info("Deleted {} records from {}", execute,
+ CONTAINER_COUNT_BY_SIZE);
+ } catch (Exception e) {
+ LOG.error("An error occurred while truncating the table {}: {}",
+ CONTAINER_COUNT_BY_SIZE, e.getMessage(), e);
+ return;
+ }
+ }
+ startTime = System.nanoTime();
+ process(containers);
+ endTime = System.nanoTime();
+ duration = endTime - startTime;
+ durationMilliseconds = duration / 1_000_000;
+ LOG.info("Elapsed Time in milli seconds for Process() execution: ",
+ (durationMilliseconds));
+ }
+ } catch (Throwable t) {
+ LOG.error("Exception in Container Size Distribution task Thread.", t);
+ if (t instanceof InterruptedException) {
+ Thread.currentThread().interrupt();
+ }
+ }
+ }
+
+
+ /**
+ * The process() function is responsible for updating the counts of
+ * containers being tracked in a containerSizeCountMap based on the
+ * ContainerInfo objects in the list containers.It then iterates through
+ * the list of containers and does the following for each container:
+ *
+ * 1) If the container is not present in processedContainers,
+ * it is a new container, so it is added to the processedContainers map
+ * and the count for its size in the containerSizeCountMap is incremented
+ * by 1 using the handlePutKeyEvent() function.
+ * 2) If the container is present in processedContainers but its size has
+ * the processedContainers map is updated to the new size and the count for
+ * the old size in the containerSizeCountMap is decremented by 1 using the
+ * handleDeleteKeyEvent() function. The count for the new size is then
+ * incremented by 1 using the handlePutKeyEvent() function.
+ * 3) If the container is present in both processedContainers and containers,
Review Comment:
Pls change the description and defintion of deleted containers.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/UtilizationEndpoint.java:
##########
@@ -98,4 +107,37 @@ public Response getFileCounts(
}
return Response.ok(resultSet).build();
}
+
+ /**
+ * Return the container size counts from Recon DB.
+ *
+ * @return {@link Response}
+ */
+ @GET
+ @Path("/containerCount")
+ public Response getContainerCounts(
+ @QueryParam(RECON_QUERY_CONTAINER_SIZE)
+ long containerSize
Review Comment:
Pls change the name of parameter "containerSize" to "upperBound"
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerSizeCountTask.java:
##########
@@ -0,0 +1,371 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.ozone.recon.ReconUtils;
+import org.apache.hadoop.ozone.recon.scm.ReconScmTask;
+import org.apache.hadoop.ozone.recon.spi.StorageContainerServiceProvider;
+import org.hadoop.ozone.recon.schema.UtilizationSchemaDefinition;
+import org.hadoop.ozone.recon.schema.tables.daos.ContainerCountBySizeDao;
+import org.hadoop.ozone.recon.schema.tables.daos.ReconTaskStatusDao;
+import org.hadoop.ozone.recon.schema.tables.pojos.ContainerCountBySize;
+import org.jooq.DSLContext;
+import org.jooq.Record1;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+import static org.hadoop.ozone.recon.schema.tables.ContainerCountBySizeTable.CONTAINER_COUNT_BY_SIZE;
+
+
+/**
+ * Class that scans the list of containers and keeps track of container sizes
+ * binned into ranges (1KB, 2Kb…,4MB,…1GB,…5GB…) to the Recon
+ * containerSize DB.
+ */
+public class ContainerSizeCountTask extends ReconScmTask {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ContainerSizeCountTask.class);
+
+ private StorageContainerServiceProvider scmClient;
+ private ContainerManager containerManager;
+ private final long interval;
+ private ContainerCountBySizeDao containerCountBySizeDao;
+ private DSLContext dslContext;
+ private HashMap<ContainerID, Long> processedContainers = new HashMap<>();
+ private ReadWriteLock lock = new ReentrantReadWriteLock(true);
+
+ public ContainerSizeCountTask(
+ ContainerManager containerManager,
+ StorageContainerServiceProvider scmClient,
+ ReconTaskStatusDao reconTaskStatusDao,
+ ReconTaskConfig reconTaskConfig,
+ ContainerCountBySizeDao containerCountBySizeDao,
+ UtilizationSchemaDefinition utilizationSchemaDefinition) {
+ super(reconTaskStatusDao);
+ this.scmClient = scmClient;
+ this.containerManager = containerManager;
+ this.containerCountBySizeDao = containerCountBySizeDao;
+ this.dslContext = utilizationSchemaDefinition.getDSLContext();
+ interval = reconTaskConfig.getContainerSizeCountTaskInterval().toMillis();
+ }
+
+
+ /**
+ * The run() method is the main loop of the ContainerSizeCountTask class.
+ * It periodically retrieves a list of containers from the containerManager,
+ * and then calls either to reprocess() or process() method depending on
+ * whether the processedContainers map is empty or not.
+ */
+ @Override
+ protected synchronized void run() {
+ try {
+ while (canRun()) {
+ wait(interval);
+ long startTime, endTime, duration, durationMilliseconds;
+ final List<ContainerInfo> containers = containerManager.getContainers();
+ if (processedContainers.isEmpty()) {
+ try {
+ int execute =
+ dslContext.truncate(CONTAINER_COUNT_BY_SIZE).execute();
+ LOG.info("Deleted {} records from {}", execute,
+ CONTAINER_COUNT_BY_SIZE);
+ } catch (Exception e) {
+ LOG.error("An error occurred while truncating the table {}: {}",
+ CONTAINER_COUNT_BY_SIZE, e.getMessage(), e);
+ return;
+ }
+ }
+ startTime = System.nanoTime();
+ process(containers);
+ endTime = System.nanoTime();
+ duration = endTime - startTime;
+ durationMilliseconds = duration / 1_000_000;
+ LOG.info("Elapsed Time in milli seconds for Process() execution: ",
+ (durationMilliseconds));
+ }
+ } catch (Throwable t) {
+ LOG.error("Exception in Container Size Distribution task Thread.", t);
+ if (t instanceof InterruptedException) {
+ Thread.currentThread().interrupt();
+ }
+ }
+ }
+
+
+ /**
+ * The process() function is responsible for updating the counts of
+ * containers being tracked in a containerSizeCountMap based on the
+ * ContainerInfo objects in the list containers.It then iterates through
+ * the list of containers and does the following for each container:
+ *
+ * 1) If the container is not present in processedContainers,
+ * it is a new container, so it is added to the processedContainers map
+ * and the count for its size in the containerSizeCountMap is incremented
+ * by 1 using the handlePutKeyEvent() function.
+ * 2) If the container is present in processedContainers but its size has
+ * the processedContainers map is updated to the new size and the count for
+ * the old size in the containerSizeCountMap is decremented by 1 using the
+ * handleDeleteKeyEvent() function. The count for the new size is then
+ * incremented by 1 using the handlePutKeyEvent() function.
+ * 3) If the container is present in both processedContainers and containers,
+ * it means the container has not been deleted. Therefore, it is removed
+ * from the deletedContainers map.
+ *
+ * The remaining containers inside the deletedContainers map are the ones
+ * that are not in the cluster and need to be deleted. Finally, the counts in
+ * the containerSizeCountMap are written to the database using the
+ * writeCountsToDB() function.
+ */
+ public void process(List<ContainerInfo> containers) {
+ lock.writeLock().lock();
+ try {
+ HashMap<ContainerSizeCountKey, Long> containerSizeCountMap =
+ new HashMap<>();
+ HashMap<ContainerID, Long> deletedContainers = new HashMap<>();
+ deletedContainers.putAll(processedContainers);
+
+ // Loop to handle container create and size-update operations
+ for (ContainerInfo container : containers) {
+ // The containers present in the cache hence it is not yet deleted
+ deletedContainers.remove(container.containerID());
+ // For New Container being created
+ if (!processedContainers.containsKey(container.containerID())) {
+ incrementContainerSizeCount(container.getUsedBytes(),
+ containerSizeCountMap);
+ processedContainers.put(container.containerID(),
+ container.getUsedBytes());
+ } else if (processedContainers.get(container.containerID()) !=
+ container.getUsedBytes()) { // If the Container Size is Updated
+ decrementContainerSizeCount(
+ processedContainers.get(container.containerID()),
+ containerSizeCountMap);
+ incrementContainerSizeCount(container.getUsedBytes(),
+ containerSizeCountMap);
+ processedContainers.put(container.containerID(),
+ container.getUsedBytes());
+ }
+ }
+
+ // Method to handle Container delete operations
+ handleContainerDeleteOperations(deletedContainers, containerSizeCountMap);
+
+ // Write to the database
+ writeCountsToDB(false, containerSizeCountMap);
+ containerSizeCountMap.clear();
+ LOG.info("Completed a 'process' run of ContainerSizeCountTask.");
+ } finally {
+ lock.writeLock().unlock();
+ }
+ }
+
+
+ /**
+ * Populate DB with the counts of container sizes calculated
+ * using the dao.
+ * <p>
+ * The writeCountsToDB function updates the database with the count of
+ * container sizes. It does this by creating two lists of records to be
+ * inserted or updated in the database. It iterates over the keys of the
+ * containerSizeCountMap and creates a new record for each key. It then
+ * checks whether the database has been truncated or not. If it has not been
+ * truncated, it attempts to find the current count for the container size
+ * in the database and either inserts a new record or updates the current
+ * record with the updated count. If the database has been truncated,
+ * it only inserts a new record if the count is non-zero. Finally, it
+ * uses the containerCountBySizeDao to insert the new records and update
+ * the existing records in the database.
+ *
+ * @param isDbTruncated that checks if the database has been truncated or not.
+ * @param containerSizeCountMap stores counts of container sizes
+ */
+ private void writeCountsToDB(boolean isDbTruncated,
+ Map<ContainerSizeCountKey, Long>
+ containerSizeCountMap) {
+ List<ContainerCountBySize> insertToDb = new ArrayList<>();
+ List<ContainerCountBySize> updateInDb = new ArrayList<>();
+
+ containerSizeCountMap.keySet().forEach((ContainerSizeCountKey key) -> {
+ ContainerCountBySize newRecord = new ContainerCountBySize();
+ newRecord.setContainerSize(key.containerSizeUpperBound);
+ newRecord.setCount(containerSizeCountMap.get(key));
+ if (!isDbTruncated) {
+ // Get the current count from database and update
+ Record1<Long> recordToFind =
+ dslContext.newRecord(
+ CONTAINER_COUNT_BY_SIZE.CONTAINER_SIZE)
+ .value1(key.containerSizeUpperBound);
+ ContainerCountBySize containerCountRecord =
+ containerCountBySizeDao.findById(recordToFind.value1());
+ if (containerCountRecord == null && newRecord.getCount() > 0L) {
+ // insert new row only for non-zero counts.
+ insertToDb.add(newRecord);
+ } else if (containerCountRecord != null) {
+ newRecord.setCount(containerCountRecord.getCount() +
+ containerSizeCountMap.get(key));
+ updateInDb.add(newRecord);
+ }
+ } else if (newRecord.getCount() > 0) {
+ // insert new row only for non-zero counts.
+ insertToDb.add(newRecord);
+ }
+ });
+ containerCountBySizeDao.insert(insertToDb);
+ containerCountBySizeDao.update(updateInDb);
+ }
+
+ @Override
+ public String getTaskName() {
+ return "ContainerSizeCountTask";
+ }
+
+ /**
+ *
+ * The handleContainerDeleteOperations() function loops through the entries
+ * in the deletedContainers map and calls the handleDeleteKeyEvent() function
+ * for each one. This will decrement the size counts of those containers by
+ * one which are no longer present in the cluster
+ *
+ * Used by process()
+ *
+ * @param deletedContainers Map that stores the info of deleted containers
+ * @param containerSizeCountMap Map that stores the counts of container sizes
+ */
+ private void handleContainerDeleteOperations(
+ Map<ContainerID, Long> deletedContainers,
+ Map<ContainerSizeCountKey, Long> containerSizeCountMap) {
+ for (Map.Entry<ContainerID, Long> containerId :
+ deletedContainers.entrySet()) {
+ long containerSize = deletedContainers.get(containerId.getKey());
+ decrementContainerSizeCount(containerSize, containerSizeCountMap);
+ }
+ }
+
+ /**
+ * Calculate and update the count of containers being tracked by
+ * containerSizeCountMap.
+ *
+ * The function calculates the upper size bound of the size range that the
+ * given container size belongs to, using the getContainerSizeCountKey
+ * function. It then increments the count of containers belonging to that
+ * size range by 1. If the map does not contain an entry for the calculated
+ * size range, the count is set to +1. The updated count is then stored in
+ * the map under the calculated size range. This function is used to handle
+ * a create event, i.e., when a container is created in the cluster.
+ *
+ * Used by process().
+ *
+ * @param containerSize to calculate the upperSizeBound
+ */
+ private void incrementContainerSizeCount(long containerSize,
+ Map<ContainerSizeCountKey, Long> containerSizeCountMap) {
+ ContainerSizeCountKey key = getContainerSizeCountKey(containerSize);
+ Long count = containerSizeCountMap.containsKey(key) ?
+ containerSizeCountMap.get(key) + 1L : 1L;
+ containerSizeCountMap.put(key, count);
+ }
+
+ /**
+ * Calculate and update the count of container being tracked by
+ * containerSizeCountMap.
+ *
+ * The function calculates the upper size bound of the size range that the
+ * given container size belongs to, using the getContainerSizeCountKey
+ * function. It then decrements the count of containers belonging to that
+ * size range by 1. If the map does not contain an entry for the calculated
+ * size range, the count is set to -1. The updated count is then stored in
+ * the map under the calculated size range. This function is used to handle
+ * a delete event, i.e., when a container is deleted from the cluster.
+ *
+ * Used by process().
+ *
+ * @param containerSize to calculate the upperSizeBound
+ */
+ private void decrementContainerSizeCount(long containerSize,
+ Map<ContainerSizeCountKey, Long> containerSizeCountMap) {
+ ContainerSizeCountKey key = getContainerSizeCountKey(containerSize);
+ Long count = containerSizeCountMap.containsKey(key) ?
+ containerSizeCountMap.get(key) - 1L : -1L;
+ containerSizeCountMap.put(key, count);
+ }
+
+ /**
+ *
+ * The purpose of this function is to categorize containers into different
+ * size ranges, or "bins," based on their size.
+ * The ContainerSizeCountKey object is used to store the upper bound value
+ * for each size range, and is later used to lookup the count of containers
+ * in that size range within a Map.
+ *
+ * Used by decrementContainerSizeCount() and incrementContainerSizeCount()
+ *
+ * @param containerSize to calculate the upperSizeBound
+ */
+ private ContainerSizeCountKey getContainerSizeCountKey(
+ long containerSize) {
+ return new ContainerSizeCountKey(
+ // Using the FileSize UpperBound Calculator for now, we can replace it
Review Comment:
Pls change the description, not to use FileSize upper bound
--
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] dombizita commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1166699124
##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/persistence/TestUtilizationSchemaDefinition.java:
##########
@@ -103,6 +104,21 @@ public void testReconSchemaCreated() throws Exception {
"Unexpected number of columns");
assertEquals(expectedPairsFileCount, actualPairsFileCount,
"Columns Do not Match ");
+
+
+ ResultSet resultSetContainerCount = metaData.getColumns(null, null,
+ CONTAINER_COUNT_BY_SIZE_TABLE_NAME, null);
+
Review Comment:
maybe add a similar `expectedPairsContainerCount` here for the expected columns, like for the file count by size on line 87-95. also add the check if the columns match (like on line 105).
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/UtilizationEndpoint.java:
##########
@@ -98,4 +111,44 @@ public Response getFileCounts(
}
return Response.ok(resultSet).build();
}
+
+ /**
+ * Return the container size counts from Recon DB.
+ *
+ * @return {@link Response}
+ */
+ @GET
+ @Path("/containerCount")
+ public Response getContainerCounts(
+ @QueryParam(RECON_QUERY_CONTAINER_SIZE)
+ long containerSize) {
+ DSLContext dslContext = utilizationSchemaDefinition.getDSLContext();
+ Long containerSizeUpperBound =
+ ReconUtils.getContainerSizeUpperBound(containerSize);
+ List<ContainerCountBySize> resultSet;
+ try {
+ if (containerSize > 0) {
+ // Get the current count from database and update
Review Comment:
what do you mean by "update" in the comment?
##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/tasks/TestContainerSizeCountTask.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+import static org.hadoop.ozone.recon.schema.tables.ContainerCountBySizeTable.CONTAINER_COUNT_BY_SIZE;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.mock;
+
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.ozone.recon.persistence.AbstractReconSqlDBTest;
+import org.apache.hadoop.ozone.recon.spi.StorageContainerServiceProvider;
+import org.hadoop.ozone.recon.schema.UtilizationSchemaDefinition;
+import org.hadoop.ozone.recon.schema.tables.daos.ContainerCountBySizeDao;
+import org.hadoop.ozone.recon.schema.tables.daos.ReconTaskStatusDao;
+import org.jooq.DSLContext;
+import org.jooq.Record1;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Class to test a process and reprocess methods of ContainerSizeCountTask.
Review Comment:
please update documentation as there is no reprocess method anymore
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconIncrementalContainerReportHandler.java:
##########
@@ -99,5 +103,6 @@ public void onMessage(final IncrementalContainerReportFromDatanode report,
}
}
containerManager.notifyContainerReportProcessing(false, success);
+ containerSizeCountTask.process(containerManager.getContainers());
Review Comment:
here the same: this means that the ContainerSizeCountTask is possible to run more times in each defined interval? if we are calling the `process()` here too.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconDeadNodeHandler.java:
##########
@@ -44,17 +45,22 @@ public class ReconDeadNodeHandler extends DeadNodeHandler {
private StorageContainerServiceProvider scmClient;
private ContainerHealthTask containerHealthTask;
private PipelineSyncTask pipelineSyncTask;
+ private ContainerSizeCountTask containerSizeCountTask;
+ private ContainerManager containerManager;
public ReconDeadNodeHandler(NodeManager nodeManager,
PipelineManager pipelineManager,
ContainerManager containerManager,
StorageContainerServiceProvider scmClient,
ContainerHealthTask containerHealthTask,
- PipelineSyncTask pipelineSyncTask) {
+ PipelineSyncTask pipelineSyncTask,
+ ContainerSizeCountTask containerSizeCountTask) {
super(nodeManager, pipelineManager, containerManager);
this.scmClient = scmClient;
+ this.containerManager = containerManager;
Review Comment:
what was the conclusion here?
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ReconTaskConfig.java:
##########
@@ -83,4 +83,23 @@ public Duration getSafeModeWaitThreshold() {
public void setSafeModeWaitThreshold(Duration safeModeWaitThreshold) {
this.safeModeWaitThreshold = safeModeWaitThreshold.toMillis();
}
+
+ @Config(key = "containercounttask.interval",
+ type = ConfigType.TIME,
+ defaultValue = "60s",
+ tags = { ConfigTag.RECON, ConfigTag.OZONE },
+ description = "The time interval of the periodic check for " +
+ "container size distribution in the cluster as reported "
Review Comment:
is this sentence unfinished?
```suggestion
description = "The time interval of the periodic check for " +
"container size distribution in the cluster."
```
##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/scm/TestReconIncrementalContainerReportHandler.java:
##########
@@ -133,6 +137,8 @@ public void testProcessICRStateMismatch()
containerWithPipeline.getPipeline().getFirstNode();
NodeManager nodeManagerMock = mock(NodeManager.class);
when(nodeManagerMock.getNodeByUuid(any())).thenReturn(datanodeDetails);
+ ContainerSizeCountTask containerSizeCountTask =
+ mock(ContainerSizeCountTask.class);
Review Comment:
I also think this would be great but I don't think the task has any methods that we can try here.
nit (if you do changes here, otherwise you can leave it like it is).
```suggestion
ContainerSizeCountTask containerSizeCountTaskMock =
mock(ContainerSizeCountTask.class);
```
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerSizeCountTask.java:
##########
@@ -0,0 +1,367 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.ozone.recon.ReconUtils;
+import org.apache.hadoop.ozone.recon.scm.ReconScmTask;
+import org.apache.hadoop.ozone.recon.spi.StorageContainerServiceProvider;
+import org.hadoop.ozone.recon.schema.UtilizationSchemaDefinition;
+import org.hadoop.ozone.recon.schema.tables.daos.ContainerCountBySizeDao;
+import org.hadoop.ozone.recon.schema.tables.daos.ReconTaskStatusDao;
+import org.hadoop.ozone.recon.schema.tables.pojos.ContainerCountBySize;
+import org.jooq.DSLContext;
+import org.jooq.Record1;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+import static org.hadoop.ozone.recon.schema.tables.ContainerCountBySizeTable.CONTAINER_COUNT_BY_SIZE;
+
+
+/**
+ * Class that scans the list of containers and keeps track of container sizes
+ * binned into ranges (1KB, 2Kb…,4MB,…1GB,…5GB…) to the Recon
+ * containerSize DB.
+ */
+public class ContainerSizeCountTask extends ReconScmTask {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ContainerSizeCountTask.class);
+
+ private StorageContainerServiceProvider scmClient;
+ private ContainerManager containerManager;
+ private final long interval;
+ private ContainerCountBySizeDao containerCountBySizeDao;
+ private DSLContext dslContext;
+ private HashMap<ContainerID, Long> processedContainers = new HashMap<>();
+ private ReadWriteLock lock = new ReentrantReadWriteLock(true);
+
+ public ContainerSizeCountTask(
+ ContainerManager containerManager,
+ StorageContainerServiceProvider scmClient,
+ ReconTaskStatusDao reconTaskStatusDao,
+ ReconTaskConfig reconTaskConfig,
+ ContainerCountBySizeDao containerCountBySizeDao,
+ UtilizationSchemaDefinition utilizationSchemaDefinition) {
+ super(reconTaskStatusDao);
+ this.scmClient = scmClient;
+ this.containerManager = containerManager;
+ this.containerCountBySizeDao = containerCountBySizeDao;
+ this.dslContext = utilizationSchemaDefinition.getDSLContext();
+ interval = reconTaskConfig.getContainerSizeCountTaskInterval().toMillis();
+ }
+
+
+ /**
+ * The run() method is the main loop of the ContainerSizeCountTask class.
+ * It periodically retrieves a list of containers from the containerManager,
+ * and then calls either to reprocess() or process() method depending on
Review Comment:
please update documentation as there is no reprocess method anymore
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/UtilizationEndpoint.java:
##########
@@ -98,4 +111,44 @@ public Response getFileCounts(
}
return Response.ok(resultSet).build();
}
+
+ /**
+ * Return the container size counts from Recon DB.
+ *
+ * @return {@link Response}
+ */
+ @GET
+ @Path("/containerCount")
+ public Response getContainerCounts(
+ @QueryParam(RECON_QUERY_CONTAINER_SIZE)
+ long containerSize) {
+ DSLContext dslContext = utilizationSchemaDefinition.getDSLContext();
+ Long containerSizeUpperBound =
+ ReconUtils.getContainerSizeUpperBound(containerSize);
+ List<ContainerCountBySize> resultSet;
+ try {
+ if (containerSize > 0) {
+ // Get the current count from database and update
+ Record1<Long> recordToFind =
+ dslContext.newRecord(
+ CONTAINER_COUNT_BY_SIZE.CONTAINER_SIZE)
+ .value1(containerSizeUpperBound);
+ ContainerCountBySize record =
+ containerCountBySizeDao.findById(recordToFind.value1());
+ resultSet = record != null ?
+ Collections.singletonList(record) : Collections.emptyList();
+ } else {
+ // fetch all records having values greater than zero
Review Comment:
why don't we fetch all the records, even if a container size has 0 occurance?
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconDeadNodeHandler.java:
##########
@@ -79,6 +85,7 @@ public void onMessage(final DatanodeDetails datanodeDetails,
}
containerHealthTask.triggerContainerHealthCheck();
pipelineSyncTask.triggerPipelineSyncTask();
+ containerSizeCountTask.process(containerManager.getContainers());
Review Comment:
this means that the ContainerSizeCountTask is possible to run more times in each defined interval? if we are calling the `process()` 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.
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] ArafatKhan2198 commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1168241337
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ReconTaskConfig.java:
##########
@@ -83,4 +83,23 @@ public Duration getSafeModeWaitThreshold() {
public void setSafeModeWaitThreshold(Duration safeModeWaitThreshold) {
this.safeModeWaitThreshold = safeModeWaitThreshold.toMillis();
}
+
+ @Config(key = "containercounttask.interval",
+ type = ConfigType.TIME,
+ defaultValue = "60s",
+ tags = { ConfigTag.RECON, ConfigTag.OZONE },
+ description = "The time interval of the periodic check for " +
+ "container size distribution in the cluster as reported "
Review Comment:
Good find @dombizita Ill make the changes !!
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.
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] ArafatKhan2198 commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1168249576
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/UtilizationEndpoint.java:
##########
@@ -98,4 +111,44 @@ public Response getFileCounts(
}
return Response.ok(resultSet).build();
}
+
+ /**
+ * Return the container size counts from Recon DB.
+ *
+ * @return {@link Response}
+ */
+ @GET
+ @Path("/containerCount")
+ public Response getContainerCounts(
+ @QueryParam(RECON_QUERY_CONTAINER_SIZE)
+ long containerSize) {
+ DSLContext dslContext = utilizationSchemaDefinition.getDSLContext();
+ Long containerSizeUpperBound =
+ ReconUtils.getContainerSizeUpperBound(containerSize);
+ List<ContainerCountBySize> resultSet;
+ try {
+ if (containerSize > 0) {
+ // Get the current count from database and update
+ Record1<Long> recordToFind =
+ dslContext.newRecord(
+ CONTAINER_COUNT_BY_SIZE.CONTAINER_SIZE)
+ .value1(containerSizeUpperBound);
+ ContainerCountBySize record =
+ containerCountBySizeDao.findById(recordToFind.value1());
+ resultSet = record != null ?
+ Collections.singletonList(record) : Collections.emptyList();
+ } else {
+ // fetch all records having values greater than zero
Review Comment:
I don't think we need to consider containers with a size of zero, as they likely don't exist. Additionally, including the count of zero-sized containers in the graph may not be meaningful.
--
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] ArafatKhan2198 commented on pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on PR #4107:
URL: https://github.com/apache/ozone/pull/4107#issuecomment-1523399237
> > So after a few discussions with @nandakumar131 The conversation was about adding a trigger to the `IncrementalContainerReportHandler` in order to trigger the `ContainerCountTask` whenever the handler is executed. It was determined that the `ICRReportHandler` will be triggered frequently in a busy cluster, and adding a trigger would put additional load on the cluster. Therefore, the final decision was not to add the trigger to the handler.
> > @devmadhuu @dombizita please take a final look at this !!
>
> during the discussion did you talk about [ReconDeadNodeHandler.java](https://github.com/apache/ozone/pull/4107/files#diff-cf88738ad7edb0171749525ea362b6172a30c36e206e487d0ed916e226c8927d)? do you think it is good to trigger the task beside the periodic runs, is it helping that much in the understanding of container sizes? won't it cause frequent calls in some cases?
>
> beside this I am fine with your changes, thanks for addressing some of them and answering my questions @ArafatKhan2198! I added a few small comments to remove unnecessary code changes in some classes. I'd like to have @devmadhuu to review it as well as I am not sure if all of his previous comments have been addressed.
Thank you for your comment, @dombizita . I appreciate your feedback and concerns regarding the ReconDeadNodeHandler.java and its impact on container size understanding. However, I believe that the frequency of a node going down in a busy cluster is not very high, so triggering the task beside the periodic runs should not significantly impact performance. Additionally, I would like to inform you that @devmadhuu has already discussed with me the pending issues in an offline discussion, and he is good to go on this!
I will review your code suggestions and make any necessary changes. Thank you for taking the time to review my code.
--
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] devmadhuu commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1052948992
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerSizeCountTask.java:
##########
@@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.ozone.recon.ReconUtils;
+import org.apache.hadoop.ozone.recon.scm.ReconScmTask;
+import org.apache.hadoop.ozone.recon.spi.StorageContainerServiceProvider;
+import org.hadoop.ozone.recon.schema.tables.daos.ContainerCountBySizeDao;
+import org.hadoop.ozone.recon.schema.tables.daos.ReconTaskStatusDao;
+import org.hadoop.ozone.recon.schema.tables.pojos.ContainerCountBySize;
+import org.jooq.DSLContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.hadoop.ozone.recon.schema.tables.ContainerCountBySizeTable.CONTAINER_COUNT_BY_SIZE;
+
+
+/**
+ * Class that scans the list of containers and keeps track of container sizes
+ * binned into ranges (1KB, 2Kb..,4MB,.., 1TB,..1PB) to the Recon
+ * containerSize DB.
+ */
+public class ContainerSizeCountTask extends ReconScmTask {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ContainerSizeCountTask.class);
+
+ private StorageContainerServiceProvider scmClient;
+ private ContainerManager containerManager;
+ private final long interval;
+ private ContainerCountBySizeDao containerCountBySizeDao;
+ private DSLContext dslContext;
+
+
+ public ContainerSizeCountTask(
+ ContainerManager containerManager,
+ StorageContainerServiceProvider scmClient,
+ ReconTaskStatusDao reconTaskStatusDao,
+ ReconTaskConfig reconTaskConfig,
+ ContainerCountBySizeDao containerCountBySizeDao,
+ DSLContext dslContext) {
+ super(reconTaskStatusDao);
+ this.scmClient = scmClient;
+ this.containerManager = containerManager;
+ this.containerCountBySizeDao = containerCountBySizeDao;
+ this.dslContext = dslContext;
+ interval = reconTaskConfig.getContainerSizeCountTaskInterval().toMillis();
+ }
+
+
+ @Override
+ protected synchronized void run() {
+ try {
+ while (canRun()) {
+ wait(interval);
Review Comment:
Try to refactor the code below this line in separate method, so that it can be triggered on any other event like when a new container being added on SCM side instead of waiting till "interval" time. Can be doable in new JIRA for above scenario, however refactor to new method below this line.
--
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] ArafatKhan2198 commented on pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by GitBox <gi...@apache.org>.
ArafatKhan2198 commented on PR #4107:
URL: https://github.com/apache/ozone/pull/4107#issuecomment-1374063672
@dombizita @kerneltime @smengcl @devmadhuu @xBis7
Can you please do a review on 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.
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] ArafatKhan2198 commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1168257221
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconIncrementalContainerReportHandler.java:
##########
@@ -99,5 +103,6 @@ public void onMessage(final IncrementalContainerReportFromDatanode report,
}
}
containerManager.notifyContainerReportProcessing(false, success);
+ containerSizeCountTask.process(containerManager.getContainers());
Review Comment:
Yes @dombizita it will run more number of times then !!
--
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] ArafatKhan2198 commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1178620462
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconIncrementalContainerReportHandler.java:
##########
@@ -46,7 +46,7 @@
ReconIncrementalContainerReportHandler.class);
public ReconIncrementalContainerReportHandler(NodeManager nodeManager,
- ContainerManager containerManager, SCMContext scmContext) {
+ ContainerManager containerManager, SCMContext scmContext) {
Review Comment:
Removed 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.
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] smengcl merged pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl merged PR #4107:
URL: https://github.com/apache/ozone/pull/4107
--
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] ArafatKhan2198 commented on pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by GitBox <gi...@apache.org>.
ArafatKhan2198 commented on PR #4107:
URL: https://github.com/apache/ozone/pull/4107#issuecomment-1362552032
@devmadhuu @dombizita @smengcl Can you do a review for this please
--
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] kerneltime commented on pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #4107:
URL: https://github.com/apache/ozone/pull/4107#issuecomment-1357972181
cc @dombizita @devmadhuu please take a look.
--
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] devmadhuu commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1052946425
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerSizeCountTask.java:
##########
@@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.ozone.recon.ReconUtils;
+import org.apache.hadoop.ozone.recon.scm.ReconScmTask;
+import org.apache.hadoop.ozone.recon.spi.StorageContainerServiceProvider;
+import org.hadoop.ozone.recon.schema.tables.daos.ContainerCountBySizeDao;
+import org.hadoop.ozone.recon.schema.tables.daos.ReconTaskStatusDao;
+import org.hadoop.ozone.recon.schema.tables.pojos.ContainerCountBySize;
+import org.jooq.DSLContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.hadoop.ozone.recon.schema.tables.ContainerCountBySizeTable.CONTAINER_COUNT_BY_SIZE;
+
+
+/**
+ * Class that scans the list of containers and keeps track of container sizes
+ * binned into ranges (1KB, 2Kb..,4MB,.., 1TB,..1PB) to the Recon
+ * containerSize DB.
+ */
+public class ContainerSizeCountTask extends ReconScmTask {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ContainerSizeCountTask.class);
+
+ private StorageContainerServiceProvider scmClient;
+ private ContainerManager containerManager;
+ private final long interval;
+ private ContainerCountBySizeDao containerCountBySizeDao;
+ private DSLContext dslContext;
+
+
+ public ContainerSizeCountTask(
+ ContainerManager containerManager,
+ StorageContainerServiceProvider scmClient,
+ ReconTaskStatusDao reconTaskStatusDao,
+ ReconTaskConfig reconTaskConfig,
+ ContainerCountBySizeDao containerCountBySizeDao,
+ DSLContext dslContext) {
+ super(reconTaskStatusDao);
+ this.scmClient = scmClient;
+ this.containerManager = containerManager;
+ this.containerCountBySizeDao = containerCountBySizeDao;
+ this.dslContext = dslContext;
+ interval = reconTaskConfig.getContainerSizeCountTaskInterval().toMillis();
+ }
+
+
+ @Override
+ protected synchronized void run() {
+ try {
+ while (canRun()) {
+ wait(interval);
+
+ final List<ContainerInfo> containers = containerManager.getContainers();
+ Map<ContainerSizeCountKey, Long> containerSizeCountMap =
+ new HashMap<>();
+ for (int i = 0; i < containers.size(); i++) {
+ handlePutKeyEvent(containers.get(i), containerSizeCountMap);
Review Comment:
Try to do profile and time test this with at least 100K containers, as we are deleting whole table below and doing fresh operation every 5 mins.
--
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] smengcl commented on pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #4107:
URL: https://github.com/apache/ozone/pull/4107#issuecomment-1535720979
Thanks @ArafatKhan2198 for the improvement, @devmadhuu @dombizita for reviewing 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.
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] devmadhuu commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1064283871
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerSizeCountTask.java:
##########
@@ -0,0 +1,399 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.ozone.recon.ReconUtils;
+import org.apache.hadoop.ozone.recon.scm.ReconScmTask;
+import org.apache.hadoop.ozone.recon.spi.StorageContainerServiceProvider;
+import org.hadoop.ozone.recon.schema.UtilizationSchemaDefinition;
+import org.hadoop.ozone.recon.schema.tables.daos.ContainerCountBySizeDao;
+import org.hadoop.ozone.recon.schema.tables.daos.ReconTaskStatusDao;
+import org.hadoop.ozone.recon.schema.tables.pojos.ContainerCountBySize;
+import org.jooq.DSLContext;
+import org.jooq.Record1;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.hadoop.ozone.recon.schema.tables.ContainerCountBySizeTable.CONTAINER_COUNT_BY_SIZE;
+
+
+/**
+ * Class that scans the list of containers and keeps track of container sizes
+ * binned into ranges (1KB, 2Kb…,4MB,…1GB,…5GB…) to the Recon
+ * containerSize DB.
+ */
+public class ContainerSizeCountTask extends ReconScmTask {
Review Comment:
This task should also be called on ICR from DN, so that most upto date ContainerSize Distribution information is with Recon. Pls check how it is being done for ContainerHealthTask and PipeLineSyncTask.
--
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] ArafatKhan2198 commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1178620582
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconStorageContainerManagerFacade.java:
##########
@@ -165,9 +174,7 @@ public ReconStorageContainerManagerFacade(OzoneConfiguration conf,
this.eventQueue = new EventQueue();
eventQueue.setSilent(true);
this.scmContext = new SCMContext.Builder()
- .setIsPreCheckComplete(true)
- .setSCM(this)
- .build();
+ .setIsPreCheckComplete(true).setSCM(this).build();
Review Comment:
Yea I have removed this and and the others as well!!
--
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] ArafatKhan2198 commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1168258166
##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/tasks/TestContainerSizeCountTask.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.tasks;
+
+import static org.hadoop.ozone.recon.schema.tables.ContainerCountBySizeTable.CONTAINER_COUNT_BY_SIZE;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.mock;
+
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManager;
+import org.apache.hadoop.ozone.recon.persistence.AbstractReconSqlDBTest;
+import org.apache.hadoop.ozone.recon.spi.StorageContainerServiceProvider;
+import org.hadoop.ozone.recon.schema.UtilizationSchemaDefinition;
+import org.hadoop.ozone.recon.schema.tables.daos.ContainerCountBySizeDao;
+import org.hadoop.ozone.recon.schema.tables.daos.ReconTaskStatusDao;
+import org.jooq.DSLContext;
+import org.jooq.Record1;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Class to test a process and reprocess methods of ContainerSizeCountTask.
Review Comment:
Sure will change!!
--
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] ArafatKhan2198 commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1168281818
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconDeadNodeHandler.java:
##########
@@ -44,17 +45,22 @@ public class ReconDeadNodeHandler extends DeadNodeHandler {
private StorageContainerServiceProvider scmClient;
private ContainerHealthTask containerHealthTask;
private PipelineSyncTask pipelineSyncTask;
+ private ContainerSizeCountTask containerSizeCountTask;
+ private ContainerManager containerManager;
public ReconDeadNodeHandler(NodeManager nodeManager,
PipelineManager pipelineManager,
ContainerManager containerManager,
StorageContainerServiceProvider scmClient,
ContainerHealthTask containerHealthTask,
- PipelineSyncTask pipelineSyncTask) {
+ PipelineSyncTask pipelineSyncTask,
+ ContainerSizeCountTask containerSizeCountTask) {
super(nodeManager, pipelineManager, containerManager);
this.scmClient = scmClient;
+ this.containerManager = containerManager;
Review Comment:
We needed to trigger the process method of the `containerSizeCountTask` from the `ReconDeadNodeHandler` hence we need to **pass in a updated list of existing containers** to the process method, so as to update the `containerCounts` of those containers that are no more.
--
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] ArafatKhan2198 commented on a diff in pull request #4107: HDDS-7632. Add ability in Recon to track various Container Sizes across Ozone cluster.
Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on code in PR #4107:
URL: https://github.com/apache/ozone/pull/4107#discussion_r1168247672
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/UtilizationEndpoint.java:
##########
@@ -98,4 +111,44 @@ public Response getFileCounts(
}
return Response.ok(resultSet).build();
}
+
+ /**
+ * Return the container size counts from Recon DB.
+ *
+ * @return {@link Response}
+ */
+ @GET
+ @Path("/containerCount")
+ public Response getContainerCounts(
+ @QueryParam(RECON_QUERY_CONTAINER_SIZE)
+ long containerSize) {
+ DSLContext dslContext = utilizationSchemaDefinition.getDSLContext();
+ Long containerSizeUpperBound =
+ ReconUtils.getContainerSizeUpperBound(containerSize);
+ List<ContainerCountBySize> resultSet;
+ try {
+ if (containerSize > 0) {
+ // Get the current count from database and update
Review Comment:
I intended to update the previous count displayed to the user through the endpoint, but it seems unnecessary to add it to the message. So Ill change it
Thanks for the find!
--
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