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 7a522730e50f1..ba539fdf67480 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 @@ -51,7 +51,7 @@ public class SequentialBlockGroupIdGenerator extends SequentialNumber { } @Override // NumberGenerator - public long nextValue() { + public synchronized long nextValue() { skipTo((getCurrentValue() & ~BLOCK_GROUP_INDEX_MASK) + MAX_BLOCKS_IN_GROUP); // Make sure there's no conflict with existing random block IDs final Block b = new Block(getCurrentValue()); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestSequentialBlockGroupId.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestSequentialBlockGroupId.java index 25b2a02883578..8240cc48625ea 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestSequentialBlockGroupId.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestSequentialBlockGroupId.java @@ -22,11 +22,15 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.not; import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.spy; import java.io.IOException; +import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -172,6 +176,43 @@ public void testTriggerBlockGroupIdCollision() throws IOException { } } + /** + * Test that the values generated by blockGroup ID generator are unique, + * even if they are generated concurrently. + * @throws Exception + */ + @Test + public void testBlockGroupIdThreadSafety() throws Exception { + // Each thread use a list to store its own block group IDs. + List> blockIds = new ArrayList<>(); + List threads = new ArrayList<>(); + + for (int i = 0; i < 20; i++) { + blockIds.add(new ArrayList<>()); + threads.add(new Thread(() -> { + for (int j = 0; j < 1000; j++) { + long next = blockGrpIdGenerator.nextValue(); + blockIds.get(j).add(next); + } + })); + } + for (Thread t : threads) { + t.start(); + } + for (Thread t : threads) { + t.join(); + } + // Check if there are duplicate IDs. + Set allBlockIds = new HashSet<>(); + for (List set : blockIds) { + for (long id : set) { + if (!allBlockIds.add(id)) { + fail("Same block group id is generated!"); + } + } + } + } + /** * Test that collisions in the blockGroup ID when the id is occupied by legacy * block.