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