You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by ji...@apache.org on 2015/05/17 01:58:44 UTC
[16/50] hadoop git commit: HDFS-8228. Erasure Coding:
SequentialBlockGroupIdGenerator#nextValue may cause block id conflicts.
Contributed by Jing Zhao.
HDFS-8228. Erasure Coding: SequentialBlockGroupIdGenerator#nextValue may cause block id conflicts. Contributed by Jing Zhao.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/f5c6ec66
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/f5c6ec66
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/f5c6ec66
Branch: refs/heads/HDFS-7285
Commit: f5c6ec66b6cb19d287d69eae3aac964f79ca552b
Parents: c9627f8
Author: Zhe Zhang <zh...@apache.org>
Authored: Fri Apr 24 09:30:38 2015 -0700
Committer: Jing Zhao <ji...@apache.org>
Committed: Sat May 16 15:15:23 2015 -0700
----------------------------------------------------------------------
.../hadoop-hdfs/CHANGES-HDFS-EC-7285.txt | 3 ++
.../SequentialBlockGroupIdGenerator.java | 39 +++++++-------
.../SequentialBlockIdGenerator.java | 2 +-
.../hadoop/hdfs/TestDFSStripedInputStream.java | 57 +++++++++++---------
.../server/namenode/TestAddStripedBlocks.java | 21 ++++++++
5 files changed, 77 insertions(+), 45 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/f5c6ec66/hadoop-hdfs-project/hadoop-hdfs/CHANGES-HDFS-EC-7285.txt
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES-HDFS-EC-7285.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES-HDFS-EC-7285.txt
index 9357e23..cf41a9b 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES-HDFS-EC-7285.txt
+++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES-HDFS-EC-7285.txt
@@ -128,3 +128,6 @@
HDFS-8223. Should calculate checksum for parity blocks in DFSStripedOutputStream.
(Yi Liu via jing9)
+
+ HDFS-8228. Erasure Coding: SequentialBlockGroupIdGenerator#nextValue may cause
+ block id conflicts (Jing Zhao via Zhe Zhang)
http://git-wip-us.apache.org/repos/asf/hadoop/blob/f5c6ec66/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/SequentialBlockGroupIdGenerator.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/SequentialBlockGroupIdGenerator.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/SequentialBlockGroupIdGenerator.java
index e9e22ee..de8e379 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/SequentialBlockGroupIdGenerator.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/SequentialBlockGroupIdGenerator.java
@@ -19,9 +19,11 @@ package org.apache.hadoop.hdfs.server.blockmanagement;
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.hdfs.protocol.Block;
-import org.apache.hadoop.hdfs.protocol.HdfsConstants;
import org.apache.hadoop.util.SequentialNumber;
+import static org.apache.hadoop.hdfs.protocol.HdfsConstants.BLOCK_GROUP_INDEX_MASK;
+import static org.apache.hadoop.hdfs.protocol.HdfsConstants.MAX_BLOCKS_IN_GROUP;
+
/**
* Generate the next valid block group ID by incrementing the maximum block
* group ID allocated so far, with the first 2^10 block group IDs reserved.
@@ -34,6 +36,9 @@ import org.apache.hadoop.util.SequentialNumber;
* bits (n+2) to (64-m) represent the ID of its block group, while the last m
* bits represent its index of the group. The value m is determined by the
* maximum number of blocks in a group (MAX_BLOCKS_IN_GROUP).
+ *
+ * Note that the {@link #nextValue()} methods requires external lock to
+ * guarantee IDs have no conflicts.
*/
@InterfaceAudience.Private
public class SequentialBlockGroupIdGenerator extends SequentialNumber {
@@ -47,32 +52,30 @@ public class SequentialBlockGroupIdGenerator extends SequentialNumber {
@Override // NumberGenerator
public long nextValue() {
- // Skip to next legitimate block group ID based on the naming protocol
- while (super.getCurrentValue() % HdfsConstants.MAX_BLOCKS_IN_GROUP > 0) {
- super.nextValue();
- }
+ skipTo((getCurrentValue() & ~BLOCK_GROUP_INDEX_MASK) + MAX_BLOCKS_IN_GROUP);
// Make sure there's no conflict with existing random block IDs
- while (hasValidBlockInRange(super.getCurrentValue())) {
- super.skipTo(super.getCurrentValue() +
- HdfsConstants.MAX_BLOCKS_IN_GROUP);
+ final Block b = new Block(getCurrentValue());
+ while (hasValidBlockInRange(b)) {
+ skipTo(getCurrentValue() + MAX_BLOCKS_IN_GROUP);
+ b.setBlockId(getCurrentValue());
}
- if (super.getCurrentValue() >= 0) {
- BlockManager.LOG.warn("All negative block group IDs are used, " +
- "growing into positive IDs, " +
- "which might conflict with non-erasure coded blocks.");
+ if (b.getBlockId() >= 0) {
+ throw new IllegalStateException("All negative block group IDs are used, "
+ + "growing into positive IDs, "
+ + "which might conflict with non-erasure coded blocks.");
}
- return super.getCurrentValue();
+ return getCurrentValue();
}
/**
- *
- * @param id The starting ID of the range
+ * @param b A block object whose id is set to the starting point for check
* @return true if any ID in the range
* {id, id+HdfsConstants.MAX_BLOCKS_IN_GROUP} is pointed-to by a file
*/
- private boolean hasValidBlockInRange(long id) {
- for (int i = 0; i < HdfsConstants.MAX_BLOCKS_IN_GROUP; i++) {
- Block b = new Block(id + i);
+ private boolean hasValidBlockInRange(Block b) {
+ final long id = b.getBlockId();
+ for (int i = 0; i < MAX_BLOCKS_IN_GROUP; i++) {
+ b.setBlockId(id + i);
if (blockManager.getBlockCollection(b) != null) {
return true;
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/f5c6ec66/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/SequentialBlockIdGenerator.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/SequentialBlockIdGenerator.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/SequentialBlockIdGenerator.java
index c97de4b..6074784 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/SequentialBlockIdGenerator.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/SequentialBlockIdGenerator.java
@@ -54,7 +54,7 @@ public class SequentialBlockIdGenerator extends SequentialNumber {
b.setBlockId(super.nextValue());
}
if (b.getBlockId() < 0) {
- BlockManager.LOG.warn("All positive block IDs are used, " +
+ throw new IllegalStateException("All positive block IDs are used, " +
"wrapping to negative IDs, " +
"which might conflict with erasure coded block groups.");
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/f5c6ec66/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedInputStream.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedInputStream.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedInputStream.java
index 6af4a7f..73c7350 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedInputStream.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSStripedInputStream.java
@@ -22,10 +22,8 @@ import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hdfs.protocol.HdfsConstants;
-import org.junit.After;
import org.junit.AfterClass;
import org.junit.Assert;
-import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -40,18 +38,15 @@ public class TestDFSStripedInputStream {
private final static int cellSize = HdfsConstants.BLOCK_STRIPED_CELL_SIZE;
private final static int stripesPerBlock = 4;
static int blockSize = cellSize * stripesPerBlock;
- private int mod = 29;
static int numDNs = dataBlocks + parityBlocks + 2;
private static MiniDFSCluster cluster;
- private static Configuration conf;
@BeforeClass
public static void setup() throws IOException {
- conf = new Configuration();
+ Configuration conf = new Configuration();
conf.setLong(DFSConfigKeys.DFS_BLOCK_SIZE_KEY, blockSize);
- cluster
- = new MiniDFSCluster.Builder(conf).numDataNodes(numDNs).build();;
+ cluster = new MiniDFSCluster.Builder(conf).numDataNodes(numDNs).build();
cluster.getFileSystem().getClient().createErasureCodingZone("/", null);
fs = cluster.getFileSystem();
}
@@ -85,43 +80,56 @@ public class TestDFSStripedInputStream {
@Test
public void testFileSmallerThanOneStripe1() throws IOException {
- testOneFileUsingDFSStripedInputStream("/SmallerThanOneStripe", cellSize * dataBlocks - 1);
+ testOneFileUsingDFSStripedInputStream("/SmallerThanOneStripe",
+ cellSize * dataBlocks - 1);
}
@Test
public void testFileSmallerThanOneStripe2() throws IOException {
- testOneFileUsingDFSStripedInputStream("/SmallerThanOneStripe", cellSize + 123);
+ testOneFileUsingDFSStripedInputStream("/SmallerThanOneStripe",
+ cellSize + 123);
}
@Test
public void testFileEqualsWithOneStripe() throws IOException {
- testOneFileUsingDFSStripedInputStream("/EqualsWithOneStripe", cellSize * dataBlocks);
+ testOneFileUsingDFSStripedInputStream("/EqualsWithOneStripe",
+ cellSize * dataBlocks);
}
@Test
public void testFileMoreThanOneStripe1() throws IOException {
- testOneFileUsingDFSStripedInputStream("/MoreThanOneStripe1", cellSize * dataBlocks + 123);
+ testOneFileUsingDFSStripedInputStream("/MoreThanOneStripe1",
+ cellSize * dataBlocks + 123);
}
@Test
public void testFileMoreThanOneStripe2() throws IOException {
- testOneFileUsingDFSStripedInputStream("/MoreThanOneStripe2", cellSize * dataBlocks
- + cellSize * dataBlocks + 123);
+ testOneFileUsingDFSStripedInputStream("/MoreThanOneStripe2",
+ cellSize * dataBlocks + cellSize * dataBlocks + 123);
+ }
+
+ @Test
+ public void testLessThanFullBlockGroup() throws IOException {
+ testOneFileUsingDFSStripedInputStream("/LessThanFullBlockGroup",
+ cellSize * dataBlocks * (stripesPerBlock - 1) + cellSize);
}
@Test
public void testFileFullBlockGroup() throws IOException {
- testOneFileUsingDFSStripedInputStream("/FullBlockGroup", blockSize * dataBlocks);
+ testOneFileUsingDFSStripedInputStream("/FullBlockGroup",
+ blockSize * dataBlocks);
}
@Test
public void testFileMoreThanABlockGroup1() throws IOException {
- testOneFileUsingDFSStripedInputStream("/MoreThanABlockGroup1", blockSize * dataBlocks + 123);
+ testOneFileUsingDFSStripedInputStream("/MoreThanABlockGroup1",
+ blockSize * dataBlocks + 123);
}
@Test
public void testFileMoreThanABlockGroup2() throws IOException {
- testOneFileUsingDFSStripedInputStream("/MoreThanABlockGroup2", blockSize * dataBlocks + cellSize+ 123);
+ testOneFileUsingDFSStripedInputStream("/MoreThanABlockGroup2",
+ blockSize * dataBlocks + cellSize+ 123);
}
@@ -141,35 +149,32 @@ public class TestDFSStripedInputStream {
}
private byte getByte(long pos) {
+ final int mod = 29;
return (byte) (pos % mod + 1);
}
private void testOneFileUsingDFSStripedInputStream(String src, int writeBytes)
throws IOException {
- Path TestPath = new Path(src);
+ Path testPath = new Path(src);
byte[] bytes = generateBytes(writeBytes);
- DFSTestUtil.writeFile(fs, TestPath, new String(bytes));
+ DFSTestUtil.writeFile(fs, testPath, new String(bytes));
//check file length
- FileStatus status = fs.getFileStatus(TestPath);
+ FileStatus status = fs.getFileStatus(testPath);
long fileLength = status.getLen();
Assert.assertEquals("File length should be the same",
writeBytes, fileLength);
- DFSStripedInputStream dis = new DFSStripedInputStream(
- fs.getClient(), src, true);
- try {
+ try (DFSStripedInputStream dis =
+ new DFSStripedInputStream(fs.getClient(), src, true)) {
byte[] buf = new byte[writeBytes + 100];
int readLen = dis.read(0, buf, 0, buf.length);
readLen = readLen >= 0 ? readLen : 0;
Assert.assertEquals("The length of file should be the same to write size",
writeBytes, readLen);
for (int i = 0; i < writeBytes; i++) {
- Assert.assertEquals("Byte at i should be the same",
- getByte(i), buf[i]);
+ Assert.assertEquals("Byte at i should be the same", getByte(i), buf[i]);
}
- } finally {
- dis.close();
}
}
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/f5c6ec66/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAddStripedBlocks.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAddStripedBlocks.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAddStripedBlocks.java
index 6bb1162..d03e938 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAddStripedBlocks.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAddStripedBlocks.java
@@ -82,6 +82,27 @@ public class TestAddStripedBlocks {
}
}
+ /**
+ * Make sure the IDs of striped blocks do not conflict
+ */
+ @Test
+ public void testAllocateBlockId() throws Exception {
+ Path testPath = new Path("/testfile");
+ // create a file while allocates a new block
+ DFSTestUtil.writeFile(dfs, testPath, "hello, world!");
+ LocatedBlocks lb = dfs.getClient().getLocatedBlocks(testPath.toString(), 0);
+ final long firstId = lb.get(0).getBlock().getBlockId();
+ // delete the file
+ dfs.delete(testPath, true);
+
+ // allocate a new block, and make sure the new block's id does not conflict
+ // with the previous one
+ DFSTestUtil.writeFile(dfs, testPath, "hello again");
+ lb = dfs.getClient().getLocatedBlocks(testPath.toString(), 0);
+ final long secondId = lb.get(0).getBlock().getBlockId();
+ Assert.assertEquals(firstId + HdfsConstants.MAX_BLOCKS_IN_GROUP, secondId);
+ }
+
@Test
public void testAddStripedBlock() throws Exception {
final Path file = new Path("/file1");