errose28 commented on code in PR #6809: URL: https://github.com/apache/ozone/pull/6809#discussion_r1644899282
########## hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerScannersAbstract.java: ########## @@ -121,9 +121,6 @@ public abstract void testPreviouslyScannedContainerIsScanned() @Test public abstract void testShutdownDuringScan() throws Exception; - @Test - public abstract void testUnhealthyContainerNotRescanned() throws Exception; Review Comment: Instead of deleting this method, let's just invert its conditions to test the code change. So it would become `testUnhealthyContainerRescanned` ########## hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java: ########## @@ -958,7 +946,8 @@ public ScanResult scanMetaData() throws InterruptedException { public boolean shouldScanData() { boolean shouldScan = Review Comment: I think an allow-list like we have here is easier to follow. There's other cases like CLOSING that might be ok, although those should move to CLOSED soon. RECOVERING we would still want to skip. I think DELETED may also be a state encountered on the DN as part of the atomic container delete code but I'd need to refresh my memory on how that was implemented. -- 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-unsubscr...@ozone.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@ozone.apache.org For additional commands, e-mail: issues-h...@ozone.apache.org