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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
