ptlrs commented on code in PR #8442:
URL: https://github.com/apache/ozone/pull/8442#discussion_r2165902811
##########
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:
Further down, `instance.metrics.incNumContainersScanned();` increments the
counter even for containers which never scanned such as those in `UNHEALTHY`
state.
Should this be corrected as per the comment's intention?
##########
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:
Do we need to check if this container is being scanned currently by the
background scanner?
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestOnDemandContainerScannerIntegration.java:
##########
@@ -64,6 +66,12 @@ static Collection<ContainerCorruptions>
supportedCorruptionTypes() {
ContainerCorruptions.TRUNCATED_BLOCK);
}
+ static Collection<ContainerCorruptions> supportedCorruptionTypesForOpen() {
+ Set<ContainerCorruptions> set = EnumSet.copyOf(supportedCorruptionTypes());
+ set.remove(ContainerCorruptions.MISSING_BLOCK);
+ return set;
+ }
+
Review Comment:
Can we add a comment explaining why this corruption type is not needed.
--
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]
