Skip to content

HDDS-7923. [EC] Reconstruction is failing with IndexOutOfBoundsException #4258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kaijchen merged 3 commits into from
Feb 9, 2023
Merged

HDDS-7923. [EC] Reconstruction is failing with IndexOutOfBoundsException #4258

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
686e4d0
HDDS-7923. [EC] Reconstruction is failing with IndexOutOfBoundsException
umagangumalla Feb 8, 2023
70af3b7
Fix checkstyle
adoroszlai Feb 8, 2023
d02c12c
Fixed a test issue to reproduce the issue consistently.
umagangumalla Feb 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/ECBlockOutputStream.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ ContainerCommandResponseProto> executePutBlock(boolean close,
continue;
}
List<ChunkInfo> chunks = bd.getChunks();
if (chunks != null && chunks.get(0).hasStripeChecksum()) {
if (chunks != null && chunks.size() > 0 && chunks.get(0)
.hasStripeChecksum()) {
checksumBlockData = bd;
break;
}
Expand Down
27 changes: 17 additions & 10 deletions ...ration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestContainerCommandsEC.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,9 +349,16 @@ private static byte[] getBytesWith(int singleDigitNumber, int total) {
@MethodSource("recoverableMissingIndexes")
void testECReconstructionCoordinatorWith(List<Integer> missingIndexes)
throws Exception {
testECReconstructionCoordinator(missingIndexes);
testECReconstructionCoordinator(missingIndexes, 3);
}

@Test
void testECReconstructionWithPartialStripe()
Copy link
Member

@kaijchen kaijchen Feb 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @umamaheswararao, the fix looks good.
However, seems the test here is not covering the bug.
Could you please take another look?

Copy link
Contributor Author

@umamaheswararao umamaheswararao Feb 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screen Shot 2023-02-08 at 8 39 49 AM

It is failing when I run this test.
Interestingly I noticed that, when I run the tests together with testECReconstructionCoordinatorWithMissingIndexes135, it passes. Can you try running this test alone?

Copy link
Member

@kaijchen kaijchen Feb 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is failing when I run this test. Interestingly I noticed that, when I run the tests together with testECReconstructionCoordinatorWithMissingIndexes135, it passes. Can you try running this test alone?

Confirmed. Maybe there is interference among the tests here?

Copy link
Contributor Author

@umamaheswararao umamaheswararao Feb 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured out the problem. Test indeed tests the behavior, however because of using the inputchunks array which was might created by other tests, number of bytes written to file was more than one chunk. So, it's not meeting the expected situation when ran with other tests. If you run this test alone, it will consistently fail as input chunk initialization will happen as expected, that is just 1 chunk. I will update the patch shortly.

throws Exception {
testECReconstructionCoordinator(ImmutableList.of(4, 5), 1);
}


static Stream<List<Integer>> recoverableMissingIndexes() {
return Stream
.concat(IntStream.rangeClosed(1, 5).mapToObj(ImmutableList::of), Stream
Expand All @@ -367,7 +374,7 @@ static Stream<List<Integer>> recoverableMissingIndexes() {
public void testECReconstructionCoordinatorWithMissingIndexes135() {
InsufficientLocationsException exception =
Assert.assertThrows(InsufficientLocationsException.class, () -> {
testECReconstructionCoordinator(ImmutableList.of(1, 3, 5));
testECReconstructionCoordinator(ImmutableList.of(1, 3, 5), 3);
});

String expectedMessage =
Expand All @@ -377,8 +384,8 @@ public void testECReconstructionCoordinatorWithMissingIndexes135() {
Assert.assertEquals(expectedMessage, actualMessage);
}

private void testECReconstructionCoordinator(List<Integer> missingIndexes)
throws Exception {
private void testECReconstructionCoordinator(List<Integer> missingIndexes,
int numInputChunks) throws Exception {
ObjectStore objectStore = rpcClient.getObjectStore();
String keyString = UUID.randomUUID().toString();
String volumeName = UUID.randomUUID().toString();
Expand All @@ -389,7 +396,7 @@ private void testECReconstructionCoordinator(List<Integer> missingIndexes)
OzoneBucket bucket = volume.getBucket(bucketName);
XceiverClientManager xceiverClientManager =
new XceiverClientManager(config);
createKeyAndWriteData(keyString, bucket);
createKeyAndWriteData(keyString, bucket, numInputChunks);
ECReconstructionCoordinator coordinator =
new ECReconstructionCoordinator(config, certClient,
null, ECReconstructionMetrics.create());
Expand Down Expand Up @@ -499,15 +506,15 @@ private void testECReconstructionCoordinator(List<Integer> missingIndexes)
Assertions.assertEquals(metrics.getReconstructionTotal(), 1L);
}

private void createKeyAndWriteData(String keyString, OzoneBucket bucket)
throws IOException {
for (int i = 0; i < EC_DATA; i++) {
private void createKeyAndWriteData(String keyString, OzoneBucket bucket,
int numChunks) throws IOException {
for (int i = 0; i < numChunks; i++) {
inputChunks[i] = getBytesWith(i + 1, EC_CHUNK_SIZE);
}
try (OzoneOutputStream out = bucket.createKey(keyString, 4096,
new ECReplicationConfig(3, 2, EcCodec.RS, 1024), new HashMap<>())) {
Assert.assertTrue(out.getOutputStream() instanceof KeyOutputStream);
for (int i = 0; i < inputChunks.length; i++) {
for (int i = 0; i < numChunks; i++) {
out.write(inputChunks[i]);
}
}
Expand All @@ -525,7 +532,7 @@ public void testECReconstructionCoordinatorShouldCleanupContainersOnFailure()
objectStore.getVolume(volumeName).createBucket(bucketName);
OzoneVolume volume = objectStore.getVolume(volumeName);
OzoneBucket bucket = volume.getBucket(bucketName);
createKeyAndWriteData(keyString, bucket);
createKeyAndWriteData(keyString, bucket, 3);

OzoneKeyDetails key = bucket.getKey(keyString);
long conID = key.getOzoneKeyLocations().get(0).getContainerID();
Expand Down