This is an automated email from the ASF dual-hosted git repository.
umamahesh pushed a commit to branch HDDS-3816-ec
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/HDDS-3816-ec by this push:
new 22e3482 HDDS-6258. EC: Read with stopped but not dead nodes gives
IllegalStateException rather than InsufficientNodesException (#3048)
22e3482 is described below
commit 22e34820cf228b23cc648c5833b63c8ed3523481
Author: Stephen O'Donnell <[email protected]>
AuthorDate: Thu Feb 10 07:42:09 2022 +0000
HDDS-6258. EC: Read with stopped but not dead nodes gives
IllegalStateException rather than InsufficientNodesException (#3048)
---
.../hadoop/ozone/client/io/ECBlockInputStream.java | 15 ++++++--
.../ozone/client/io/ECBlockInputStreamProxy.java | 16 ++++----
.../io/ECBlockReconstructedStripeInputStream.java | 2 +-
.../ozone/client/rpc/read/ECStreamTestUtil.java | 9 +++++
.../client/rpc/read/TestECBlockInputStream.java | 23 +++++++++++
.../rpc/read/TestECBlockInputStreamProxy.java | 16 +++++---
.../TestECBlockReconstructedStripeInputStream.java | 45 ++++++++++++++++++++++
7 files changed, 109 insertions(+), 17 deletions(-)
diff --git
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockInputStream.java
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockInputStream.java
index 5212eea..b71d475 100644
---
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockInputStream.java
+++
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockInputStream.java
@@ -78,9 +78,18 @@ public class ECBlockInputStream extends
BlockExtendedInputStream {
return stripeSize;
}
- protected int availableDataLocations() {
+ /**
+ * Returns the number of available data locations, taking account of the
+ * expected number of locations. Eg, if the block is less than 1 EC chunk,
+ * we only expect 1 data location. If it is between 1 and 2 chunks, we expect
+ * there to be 2 locations, and so on.
+ * @param expectedLocations The maximum number of allowed data locations,
+ * depending on the block size.
+ * @return The number of available data locations.
+ */
+ protected int availableDataLocations(int expectedLocations) {
int count = 0;
- for (int i = 0; i < repConfig.getData(); i++) {
+ for (int i = 0; i < repConfig.getData() && i < expectedLocations; i++) {
if (dataLocations[i] != null) {
count++;
}
@@ -127,7 +136,7 @@ public class ECBlockInputStream extends
BlockExtendedInputStream {
// must have all data_num locations.
// We only consider data locations here.
int expectedDataBlocks = calculateExpectedDataBlocks(repConfig);
- return expectedDataBlocks == availableDataLocations();
+ return expectedDataBlocks == availableDataLocations(expectedDataBlocks);
}
protected int calculateExpectedDataBlocks(ECReplicationConfig rConfig) {
diff --git
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockInputStreamProxy.java
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockInputStreamProxy.java
index 2f7680e..ecde9c6 100644
---
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockInputStreamProxy.java
+++
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockInputStreamProxy.java
@@ -74,19 +74,21 @@ public class ECBlockInputStreamProxy extends
BlockExtendedInputStream {
}
/**
- * From ECReplicationConfig and Pipeline with the block locations and
location
- * indexes, determine the number of data locations available.
- * @param repConfig The EC Replication Config
+ * From the Pipeline and expected number of locations, determine the number
+ * of data locations available.
* @param pipeline The pipeline for the data block, givings its locations and
* the index of each location.
+ * @param expectedLocs The number of locations we expect for the block to
have
+ * based on its block length and replication config. The
+ * max value should be the repConfig data number.
* @return The number of locations available
*/
- public static int availableDataLocations(ECReplicationConfig repConfig,
- Pipeline pipeline) {
+ public static int availableDataLocations(Pipeline pipeline,
+ int expectedLocs) {
Set<Integer> locations = new HashSet<>();
for (DatanodeDetails dn : pipeline.getNodes()) {
int index = pipeline.getReplicaIndex(dn);
- if (index > 0 && index <= repConfig.getData()) {
+ if (index > 0 && index <= expectedLocs) {
locations.add(index);
}
}
@@ -110,7 +112,7 @@ public class ECBlockInputStreamProxy extends
BlockExtendedInputStream {
private synchronized void setReaderType() {
int expected = expectedDataLocations(repConfig, getLength());
- int available = availableDataLocations(repConfig, blockInfo.getPipeline());
+ int available = availableDataLocations(blockInfo.getPipeline(), expected);
reconstructionReader = available < expected;
}
diff --git
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockReconstructedStripeInputStream.java
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockReconstructedStripeInputStream.java
index e3fa0a7..26fbd15 100644
---
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockReconstructedStripeInputStream.java
+++
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockReconstructedStripeInputStream.java
@@ -558,7 +558,7 @@ public class ECBlockReconstructedStripeInputStream extends
ECBlockInputStream {
ECReplicationConfig repConfig = getRepConfig();
int expectedDataBlocks = calculateExpectedDataBlocks(repConfig);
int availableLocations =
- availableDataLocations() + availableParityLocations();
+ availableDataLocations(expectedDataBlocks) +
availableParityLocations();
int paddedLocations = repConfig.getData() - expectedDataBlocks;
int failedLocations = failedDataIndexes.size();
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/read/ECStreamTestUtil.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/read/ECStreamTestUtil.java
index c2f51c2..f1f4a44 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/read/ECStreamTestUtil.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/read/ECStreamTestUtil.java
@@ -221,6 +221,8 @@ public final class ECStreamTestUtil {
private List<TestBlockInputStream> blockStreams = new ArrayList<>();
private List<ByteBuffer> blockStreamData;
+ // List of EC indexes that should fail immediately on read
+ private List<Integer> failIndexes = new ArrayList<>();
private Pipeline currentPipeline;
@@ -236,6 +238,10 @@ public final class ECStreamTestUtil {
this.currentPipeline = pipeline;
}
+ public void setFailIndexes(List<Integer> fail) {
+ failIndexes.addAll(fail);
+ }
+
public BlockExtendedInputStream create(ReplicationConfig repConfig,
OmKeyLocationInfo blockInfo, Pipeline pipeline,
Token<OzoneBlockTokenIdentifier> token, boolean verifyChecksum,
@@ -246,6 +252,9 @@ public final class ECStreamTestUtil {
TestBlockInputStream stream = new TestBlockInputStream(
blockInfo.getBlockID(), blockInfo.getLength(),
blockStreamData.get(repInd - 1), repInd);
+ if (failIndexes.contains(repInd)) {
+ stream.setShouldError(true);
+ }
blockStreams.add(stream);
return stream;
}
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/read/TestECBlockInputStream.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/read/TestECBlockInputStream.java
index d681c61..60a3a85 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/read/TestECBlockInputStream.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/read/TestECBlockInputStream.java
@@ -230,6 +230,29 @@ public class TestECBlockInputStream {
}
}
+ /**
+ * This test is to ensure we can read a small key of 1 chunk or less when
only
+ * the first replica index is available.
+ */
+ @Test
+ public void testSimpleReadUnderOneChunk() throws IOException {
+ OmKeyLocationInfo keyInfo =
+ ECStreamTestUtil.createKeyInfo(repConfig, 1, ONEMB);
+ try (ECBlockInputStream ecb = new ECBlockInputStream(repConfig,
+ keyInfo, true, null, null, streamFactory)) {
+
+ ByteBuffer buf = ByteBuffer.allocate(100);
+
+ int read = ecb.read(buf);
+ Assert.assertEquals(100, read);
+ validateBufferContents(buf, 0, 100, (byte) 0);
+ Assert.assertEquals(100, ecb.getPos());
+ }
+ for (TestBlockInputStream s : streamFactory.getBlockStreams()) {
+ Assert.assertTrue(s.isClosed());
+ }
+ }
+
@Test
public void testReadPastEOF() throws IOException {
OmKeyLocationInfo keyInfo =
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/read/TestECBlockInputStreamProxy.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/read/TestECBlockInputStreamProxy.java
index f9b5f38..160ded4 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/read/TestECBlockInputStreamProxy.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/read/TestECBlockInputStreamProxy.java
@@ -92,21 +92,25 @@ public class TestECBlockInputStreamProxy {
@Test
public void testAvailableDataLocations() {
Map<DatanodeDetails, Integer> dnMap =
- ECStreamTestUtil.createIndexMap(1, 2, 4, 5);
+ ECStreamTestUtil.createIndexMap(1, 2, 3, 4, 5);
OmKeyLocationInfo blockInfo =
ECStreamTestUtil.createKeyInfo(repConfig, 1024, dnMap);
+ Assert.assertEquals(1, ECBlockInputStreamProxy.availableDataLocations(
+ blockInfo.getPipeline(), 1));
Assert.assertEquals(2, ECBlockInputStreamProxy.availableDataLocations(
- repConfig, blockInfo.getPipeline()));
+ blockInfo.getPipeline(), 2));
+ Assert.assertEquals(3, ECBlockInputStreamProxy.availableDataLocations(
+ blockInfo.getPipeline(), 3));
dnMap = ECStreamTestUtil.createIndexMap(1, 4, 5);
blockInfo = ECStreamTestUtil.createKeyInfo(repConfig, 1024, dnMap);
Assert.assertEquals(1, ECBlockInputStreamProxy.availableDataLocations(
- repConfig, blockInfo.getPipeline()));
+ blockInfo.getPipeline(), 3));
- dnMap = ECStreamTestUtil.createIndexMap(1, 2, 3, 4, 5);
+ dnMap = ECStreamTestUtil.createIndexMap(2, 3, 4, 5);
blockInfo = ECStreamTestUtil.createKeyInfo(repConfig, 1024, dnMap);
- Assert.assertEquals(3, ECBlockInputStreamProxy.availableDataLocations(
- repConfig, blockInfo.getPipeline()));
+ Assert.assertEquals(0, ECBlockInputStreamProxy.availableDataLocations(
+ blockInfo.getPipeline(), 1));
}
@Test
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/read/TestECBlockReconstructedStripeInputStream.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/read/TestECBlockReconstructedStripeInputStream.java
index 079e944..ec9bbdc 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/read/TestECBlockReconstructedStripeInputStream.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/read/TestECBlockReconstructedStripeInputStream.java
@@ -117,6 +117,15 @@ public class TestECBlockReconstructedStripeInputStream {
((ECBlockReconstructedStripeInputStream)ecb).addFailedDatanodes(failed);
Assert.assertFalse(ecb.hasSufficientLocations());
}
+
+ // One chunk, indexes 2 and 3 are padding, but still reported in the
+ // container list. The other locations are missing so we should have
+ // insufficient locations.
+ dnMap = ECStreamTestUtil.createIndexMap(2, 3);
+ keyInfo = ECStreamTestUtil.createKeyInfo(repConfig, ONEMB, dnMap);
+ try (ECBlockInputStream ecb = createInputStream(keyInfo)) {
+ Assert.assertFalse(ecb.hasSufficientLocations());
+ }
}
@Test
@@ -528,6 +537,42 @@ public class TestECBlockReconstructedStripeInputStream {
}
}
+ @Test(expected=InsufficientLocationsException.class)
+ public void testAllLocationsFailOnFirstRead() throws IOException {
+ // This test simulates stale nodes. When the nodes are stale, but not yet
+ // dead, the locations will still be given to the client and it will try to
+ // read them, but the read will always fail.
+ // Additionally, if the key is small (less than 2 EC chunks), the locations
+ // for the indexes which are all padding will be returned to the client and
+ // this can confuse the "sufficient locations" check, resulting in a
strange
+ // error when selecting parity indexes (HDDS-6258)
+ int chunkSize = repConfig.getEcChunkSize();
+ int partialStripeSize = chunkSize;
+ int blockLength = partialStripeSize;
+ ByteBuffer[] dataBufs = allocateBuffers(repConfig.getData(), chunkSize);
+ ECStreamTestUtil
+ .randomFill(dataBufs, repConfig.getEcChunkSize(), dataGen,
blockLength);
+ ByteBuffer[] parity = generateParity(dataBufs, repConfig);
+
+ streamFactory = new TestBlockInputStreamFactory();
+ addDataStreamsToFactory(dataBufs, parity);
+ // Fail all the indexes containing data on their first read.
+ streamFactory.setFailIndexes(indexesToList(1, 4, 5));
+ // The locations contain the padded indexes, as will often be the case
+ // when containers are reported by SCM.
+ Map<DatanodeDetails, Integer> dnMap =
+ ECStreamTestUtil.createIndexMap(1, 2, 3, 4, 5);
+ OmKeyLocationInfo keyInfo = ECStreamTestUtil.createKeyInfo(repConfig,
+ blockLength, dnMap);
+ streamFactory.setCurrentPipeline(keyInfo.getPipeline());
+
+ ByteBuffer[] bufs = allocateByteBuffers(repConfig);
+ try (ECBlockReconstructedStripeInputStream ecb =
+ createInputStream(keyInfo)) {
+ ecb.readStripe(bufs);
+ }
+ }
+
@Test
public void testFailedLocationsAreNotRead() throws IOException {
// Generate the input data for 3 full stripes and generate the parity.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
