aswinshakil commented on code in PR #7396:
URL: https://github.com/apache/ozone/pull/7396#discussion_r1838914645


##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainerCheck.java:
##########
@@ -25,28 +25,167 @@
 import org.apache.hadoop.hdfs.util.Canceler;
 import org.apache.hadoop.hdfs.util.DataTransferThrottler;
 import org.apache.hadoop.ozone.container.common.helpers.BlockData;
+import org.apache.hadoop.ozone.container.common.impl.ContainerLayoutVersion;
 import org.apache.hadoop.ozone.container.common.interfaces.BlockIterator;
 import org.apache.hadoop.ozone.container.common.interfaces.DBHandle;
 import org.apache.hadoop.ozone.container.common.interfaces.ScanResult;
 import org.apache.hadoop.ozone.container.keyvalue.helpers.BlockUtils;
 import 
org.apache.hadoop.ozone.container.keyvalue.helpers.KeyValueContainerLocationUtil;
+import 
org.apache.hadoop.ozone.container.ozoneimpl.ContainerScanError.FailureType;
+import org.apache.hadoop.ozone.container.ozoneimpl.ContainerScanError;
 import 
org.apache.hadoop.ozone.container.ozoneimpl.ContainerScannerConfiguration;
+import org.apache.hadoop.ozone.container.ozoneimpl.DataScanResult;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.io.File;
 import java.io.RandomAccessFile;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
+import static 
org.apache.hadoop.ozone.container.keyvalue.ContainerCorruptions.CORRUPT_BLOCK;
+import static 
org.apache.hadoop.ozone.container.keyvalue.ContainerCorruptions.CORRUPT_CONTAINER_FILE;
+import static 
org.apache.hadoop.ozone.container.keyvalue.ContainerCorruptions.MISSING_BLOCK;
+import static 
org.apache.hadoop.ozone.container.keyvalue.ContainerCorruptions.MISSING_CHUNKS_DIR;
+import static 
org.apache.hadoop.ozone.container.keyvalue.ContainerCorruptions.MISSING_CONTAINER_DIR;
+import static 
org.apache.hadoop.ozone.container.keyvalue.ContainerCorruptions.MISSING_CONTAINER_FILE;
+import static 
org.apache.hadoop.ozone.container.keyvalue.ContainerCorruptions.MISSING_METADATA_DIR;
+import static 
org.apache.hadoop.ozone.container.keyvalue.ContainerCorruptions.TRUNCATED_BLOCK;
+import static 
org.apache.hadoop.ozone.container.keyvalue.ContainerCorruptions.TRUNCATED_CONTAINER_FILE;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.mock;
 
 /**
- * Basic sanity test for the KeyValueContainerCheck class.
+ * Test the KeyValueContainerCheck class's ability to detect container errors.
  */
 public class TestKeyValueContainerCheck
     extends TestKeyValueContainerIntegrityChecks {
 
+  private static final Logger LOG = 
LoggerFactory.getLogger(TestKeyValueContainerCheck.class);
+
+  /**
+   * Container fault injection is not supported with the old file per chunk 
layout.
+   * @return The container versions that should be tested with fault injection.
+   */
+  private static Stream<ContainerTestVersionInfo> provideContainerVersions() {
+    return ContainerTestVersionInfo.getLayoutList().stream()
+        .filter(c -> c.getLayout() != ContainerLayoutVersion.FILE_PER_CHUNK);
+  }
+
+  /**
+   * @return A matrix of the container versions that should be tested with 
fault injection paired with each type of
+   * metadata fault.
+   */
+  private static Stream<Arguments> provideMetadataCorruptions() {
+    List<ContainerCorruptions> metadataCorruptions = Arrays.asList(
+        MISSING_CHUNKS_DIR,
+        MISSING_METADATA_DIR,
+        MISSING_CONTAINER_DIR,
+        MISSING_CONTAINER_FILE,
+        CORRUPT_CONTAINER_FILE,
+        TRUNCATED_CONTAINER_FILE
+    );
+    return provideContainerVersions()
+        .flatMap(version -> metadataCorruptions.stream().map(corruption -> 
Arguments.of(version, corruption)));
+  }
+
+  /**
+   * When the scanner encounters an issue with container metadata, it should 
fail the scan immediately.
+   * Metadata is required before reading the data.
+   */
+  @ParameterizedTest
+  @MethodSource("provideMetadataCorruptions")
+  public void testExitEarlyOnMetadataError(ContainerTestVersionInfo 
versionInfo,
+      ContainerCorruptions metadataCorruption) throws Exception {
+    initTestData(versionInfo);
+    long containerID = 101;
+    int deletedBlocks = 0;
+    int normalBlocks = 3;
+    OzoneConfiguration conf = getConf();
+    ContainerScannerConfiguration c = 
conf.getObject(ContainerScannerConfiguration.class);
+    DataTransferThrottler throttler = new 
DataTransferThrottler(c.getBandwidthPerVolume());
+
+    KeyValueContainer container = createContainerWithBlocks(containerID,
+        normalBlocks, deletedBlocks, true);
+    KeyValueContainerCheck kvCheck = new KeyValueContainerCheck(conf, 
container);
+
+    DataScanResult result = kvCheck.fullCheck(throttler, null);
+    assertTrue(result.isHealthy());
+
+    // Inject a metadata and a data error.
+    metadataCorruption.applyTo(container);
+    // All other metadata failures are independent of the block files, so we 
can add a data failure later in the scan.
+    if (metadataCorruption != MISSING_CHUNKS_DIR && metadataCorruption != 
MISSING_CONTAINER_DIR) {
+      CORRUPT_BLOCK.applyTo(container);
+    }
+
+    result = kvCheck.fullCheck(throttler, null);
+    assertFalse(result.isHealthy());
+    // Scan should have failed after the first metadata error and not made it 
to the data error.
+    assertEquals(1, result.getErrors().size());
+    assertEquals(metadataCorruption.getExpectedResult(), 
result.getErrors().get(0).getFailureType());
+  }
+
+  /**
+   * When the scanner encounters an issues with container data, it should 
continue scanning to collect all issues
+   * among all blocks.
+   */
+  @ParameterizedTest
+  @MethodSource("provideContainerVersions")
+  public void testAllDataErrorsCollected(ContainerTestVersionInfo versionInfo) 
throws Exception {
+    initTestData(versionInfo);
+
+    long containerID = 101;
+    int deletedBlocks = 0;
+    int normalBlocks = 6;
+    OzoneConfiguration conf = getConf();
+    ContainerScannerConfiguration c = conf.getObject(
+        ContainerScannerConfiguration.class);

Review Comment:
   Nit: Can be moved to the previous line. 



##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/ContainerCorruptions.java:
##########
@@ -0,0 +1,232 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.container.keyvalue;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.IOUtils;
+import org.apache.hadoop.ozone.container.common.interfaces.Container;
+import org.apache.hadoop.ozone.container.ozoneimpl.ContainerScanError;
+import org.apache.ozone.test.GenericTestUtils;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
+import java.util.Arrays;
+import java.util.EnumSet;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.regex.Pattern;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Represents a type of container corruption that can be injected into a 
container for testing.
+ * Currently this class only supports file per block layout.
+ */
+public enum ContainerCorruptions {

Review Comment:
   Since this is only used in tests. We can name this something like 
`ContainerCorruptionsTestUtils`



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