From e9d12a6db82b4ad23e89c2d217d308d1121ce5a5 Mon Sep 17 00:00:00 2001 From: S O'Donnell Date: Thu, 11 Apr 2024 21:33:24 +0100 Subject: [PATCH 1/3] HDDS-10682. EC Reconstruction creates empty chunks at the end of blocks with partial stripes --- .../reconstruction/ECReconstructionCoordinator.java | 10 +++++++--- .../hdds/scm/storage/TestContainerCommandsEC.java | 11 ++++++++++- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java index 774c7d834545..a71e119bd43d 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java @@ -323,9 +323,13 @@ public void reconstructECBlockGroup(BlockLocationInfo blockLocationInfo, } // TODO: can be submitted in parallel for (int i = 0; i < bufs.length; i++) { - CompletableFuture - future = targetBlockStreams[i].write(bufs[i]); - checkFailures(targetBlockStreams[i], future); + if (bufs[i].limit() != 0) { + // If the buffer is empty, we don't need to write it as it will cause + // an empty chunk to be added to the end of the block. + CompletableFuture + future = targetBlockStreams[i].write(bufs[i]); + checkFailures(targetBlockStreams[i], future); + } bufs[i].clear(); } length -= readLen; diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java index 5254f8748c8a..666e9a52a45a 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java @@ -136,7 +136,9 @@ public class TestContainerCommandsEC { private static final int EC_CHUNK_SIZE = 1024 * 1024; private static final int STRIPE_DATA_SIZE = EC_DATA * EC_CHUNK_SIZE; private static final int NUM_DN = EC_DATA + EC_PARITY + 3; - private static byte[][] inputChunks = new byte[EC_DATA][EC_CHUNK_SIZE]; + // Data slots are EC_DATA + 1 so we can generate enough data to have a full stripe + // plus one extra chunk. + private static byte[][] inputChunks = new byte[EC_DATA + 1][EC_CHUNK_SIZE]; // Each key size will be in range [min, max), min inclusive, max exclusive private static final int[][] KEY_SIZE_RANGES = @@ -621,6 +623,13 @@ void testECReconstructionCoordinatorWithPartialStripe(List missingIndex testECReconstructionCoordinator(missingIndexes, 1); } + @ParameterizedTest + @MethodSource("recoverableMissingIndexes") + void testECReconstructionCoordinatorWithFullAndPartialStripe(List missingIndexes) + throws Exception { + testECReconstructionCoordinator(missingIndexes, 4); + } + @Test void testECReconstructParityWithPartialStripe() throws Exception { From 285e6ea4d841b28af71a8b7487b3e2dad86cfa22 Mon Sep 17 00:00:00 2001 From: S O'Donnell Date: Fri, 12 Apr 2024 09:46:32 +0100 Subject: [PATCH 2/3] Remove duplicated test --- .../hadoop/hdds/scm/storage/TestContainerCommandsEC.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java index 666e9a52a45a..b4814d7b5e5d 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java @@ -630,13 +630,6 @@ void testECReconstructionCoordinatorWithFullAndPartialStripe(List missi testECReconstructionCoordinator(missingIndexes, 4); } - @Test - void testECReconstructParityWithPartialStripe() - throws Exception { - testECReconstructionCoordinator(ImmutableList.of(4, 5), 1); - } - - static Stream> recoverableMissingIndexes() { return Stream .concat(IntStream.rangeClosed(1, 5).mapToObj(ImmutableList::of), Stream From f90c9a2f0f608b276cedd80107ca36086246e6f4 Mon Sep 17 00:00:00 2001 From: S O'Donnell Date: Fri, 12 Apr 2024 18:37:44 +0100 Subject: [PATCH 3/3] Change limit() to remaining() --- .../ec/reconstruction/ECReconstructionCoordinator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java index a71e119bd43d..8fadd19b67d3 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java @@ -323,7 +323,7 @@ public void reconstructECBlockGroup(BlockLocationInfo blockLocationInfo, } // TODO: can be submitted in parallel for (int i = 0; i < bufs.length; i++) { - if (bufs[i].limit() != 0) { + if (bufs[i].remaining() != 0) { // If the buffer is empty, we don't need to write it as it will cause // an empty chunk to be added to the end of the block. CompletableFuture