Hexiaoqiao commented on code in PR #5743:
URL: https://github.com/apache/hadoop/pull/5743#discussion_r1229152046


##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestSequentialBlockGroupId.java:
##########
@@ -172,6 +176,41 @@ 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 {
+    List<List<Long>> blockIds = new ArrayList<>();
+    List<Thread> 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();
+    }
+    Set<Long> allBlockIds = new HashSet<>();

Review Comment:
   Here will be more readable?
   ```
       Set<Long> allBlockIds = new HashSet<>();
       for (List<Long> set : blockIds) {
         for (long id : set) {
           if (!allBlockIds.add(id)) {
             fail("Same block group id is generated!");
           }
         }
       }
   ```
   Any more simple check way? Such as using `HashMap` directly?



##########
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() {
+  synchronized public long nextValue() {

Review Comment:
   `synchronized public long nextValue()` -> `public synchronized long 
nextValue()`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to