You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "errose28 (via GitHub)" <gi...@apache.org> on 2024/11/05 17:54:02 UTC
[PR] Hdds 11471 new scanner tests [ozone]
errose28 opened a new pull request, #7396:
URL: https://github.com/apache/ozone/pull/7396
## What changes were proposed in this pull request?
[HDDS-11290](https://issues.apache.org/jira/browse/HDDS-11290) refactored the container scanner to allow it to detect multiple errors within the same container. Due to the size of the change, only existing tests were used to make sure there were no regressions. This Jira will add tests for the new functionality where the scanner can detect and report back all errors that were present in a single container.
Summary of Changes:
- Add simple unit tests for `*ScanResult` objects
- Move the existing `ContainerCorruptions` class used for fault injection to its own class instead of an inner class.
- Support specifying blocks to operate on in `ContainerCorruptions` to allow multiple corruption types to be used on the same container.
- Add new tests for multiple data and metadata failures in `TestKeyValueContainerCheck`.
## What is the link to the Apache JIRA
HDDS-11471
## How was this patch tested?
New and existing tests should pass
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11471. Add new tests for container scanner detecting multiple errors in one container [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 merged PR #7396:
URL: https://github.com/apache/ozone/pull/7396
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11471. Add new tests for container scanner detecting multiple errors in one container [ozone]
Posted by "aswinshakil (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11471. Add new tests for container scanner detecting multiple errors in one container [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on PR #7396:
URL: https://github.com/apache/ozone/pull/7396#issuecomment-2487024386
I filed HDDS-11756 for this suggestion.
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11471. Add new tests for container scanner detecting multiple errors in one container [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #7396:
URL: https://github.com/apache/ozone/pull/7396#discussion_r1841289545
##########
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:
Good idea this makes sense now that it's a stand alone class.
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11471. Add new tests for container scanner detecting multiple errors in one container [ozone]
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #7396:
URL: https://github.com/apache/ozone/pull/7396#issuecomment-2484214305
This PR looks fine. I wondered if we should have an enum in code for all the container failures and reuse them in test vs. test defining their enums. We can then assert if a new failure mode is added, but no relevant test was added. This can be a separate PR.
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org