Tejaskriya commented on code in PR #8442:
URL: https://github.com/apache/ozone/pull/8442#discussion_r2169238318


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OnDemandContainerScanner.java:
##########
@@ -135,13 +137,20 @@ private static void performOnDemandScan(Container<?> 
container) {
       ContainerData containerData = container.getContainerData();
       logScanStart(containerData);
 
-      ScanResult result =
-          container.scanData(instance.throttler, instance.canceler);
-      // Metrics for skipped containers should not be updated.
-      if (result.getFailureType() == DELETED_CONTAINER) {
-        LOG.error("Container [{}] has been deleted.",
-            containerId, result.getException());
-        return;
+      ScanResult result = ScanResult.healthy();
+      if (container.shouldScanData()) {

Review Comment:
   I have added a test for a similar case now. The test is basically keeping a 
container.scanMetaData() call ongoing, when another request comes in. Both the 
background and on-demand scanners use this method to detect corrupt open 
containers. 
   The second scan is not scheduled.



##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OnDemandContainerScanner.java:
##########
@@ -135,13 +137,20 @@ private static void performOnDemandScan(Container<?> 
container) {
       ContainerData containerData = container.getContainerData();
       logScanStart(containerData);
 
-      ScanResult result =
-          container.scanData(instance.throttler, instance.canceler);
-      // Metrics for skipped containers should not be updated.
-      if (result.getFailureType() == DELETED_CONTAINER) {
-        LOG.error("Container [{}] has been deleted.",
-            containerId, result.getException());
-        return;
+      ScanResult result = ScanResult.healthy();
+      if (container.shouldScanData()) {
+        LOG.debug("Performing data scan for container {}", 
container.getContainerData().getContainerID());
+        result = container.scanData(instance.throttler, instance.canceler);
+        // Metrics for skipped containers should not be updated.

Review Comment:
   I have moved the metrics into the if-else now. Thanks for catching this! It 
would have wrongly counted the unhealthy ones.



-- 
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]

Reply via email to