errose28 commented on code in PR #8603:
URL: https://github.com/apache/ozone/pull/8603#discussion_r2198997089
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerScanHelper.java:
##########
@@ -107,6 +109,18 @@ public void handleUnhealthyScanResult(long containerID,
ScanResult result) throw
}
}
+ public void triggerVolumeScan(ContainerData containerData) {
+ HddsVolume volume = containerData.getVolume();
+ if (volume != null && !volume.isFailed()) {
+ log.info("Triggering a volume scan for volume [{}] as unhealthy
container [{}] was on it.",
+ volume.getStorageDir().getPath(), containerData.getContainerID());
+ StorageVolumeUtil.onFailure(volume);
+ } else {
+ log.warn("Cannot trigger volume scan for container {} since its volume
is null or has failed.",
Review Comment:
Can we add different logs for each case? Null case sounds like its alerting
to a possible bug, but failed case is not.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerScanHelper.java:
##########
@@ -107,6 +109,18 @@ public void handleUnhealthyScanResult(long containerID,
ScanResult result) throw
}
}
+ public void triggerVolumeScan(ContainerData containerData) {
+ HddsVolume volume = containerData.getVolume();
+ if (volume != null && !volume.isFailed()) {
+ log.info("Triggering a volume scan for volume [{}] as unhealthy
container [{}] was on it.",
Review Comment:
```suggestion
log.info("Triggering scan of volume [{}] with unhealthy container
[{}]",
```
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestBackgroundContainerDataScanner.java:
##########
@@ -133,6 +136,16 @@ public void testUnhealthyContainersDetected() throws
Exception {
verifyContainerMarkedUnhealthy(deletedContainer, never());
}
+ @Test
+ @Override
+ public void testUnhealthyContainersTriggersVolumeScan() throws Exception {
+ try (MockedStatic<StorageVolumeUtil> mockedStatic =
mockStatic(StorageVolumeUtil.class)) {
Review Comment:
Instead of `mockStatic`, each container instance in this test is initialized
with a mocked `HddsVolume`
[object](https://github.com/apache/ozone/blob/9a445ed01eb8224ebccbc8c7a2712eba36651aa6/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerScannersAbstract.java#L80).
We can assert the number of invocations of `HddsVolume#check` to verify it is
scanned.
If this ends up being too much change we can keep the static mock, but its
usually cleaner to avoid using that if possible.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerScanHelper.java:
##########
@@ -107,6 +109,18 @@ public void handleUnhealthyScanResult(long containerID,
ScanResult result) throw
}
}
+ public void triggerVolumeScan(ContainerData containerData) {
+ HddsVolume volume = containerData.getVolume();
+ if (volume != null && !volume.isFailed()) {
+ log.info("Triggering a volume scan for volume [{}] as unhealthy
container [{}] was on it.",
+ volume.getStorageDir().getPath(), containerData.getContainerID());
Review Comment:
nit. `HddsVolume` already defines a `toString` with this behavior.
```suggestion
volume, containerData.getContainerID());
```
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/BackgroundContainerMetadataScanner.java:
##########
@@ -67,6 +67,7 @@ public void scanContainer(Container<?> container)
}
if (result.hasErrors()) {
scanHelper.handleUnhealthyScanResult(containerID, result);
+ scanHelper.triggerVolumeScan(container.getContainerData());
Review Comment:
Can we move this call inside `ContainerScanHelper#handleUnhealthyScanResult`?
--
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]
