You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/06/07 08:00:31 UTC

[GitHub] [ozone] elek opened a new pull request #2306: Enhance freon streaing generator to support multiple threads

elek opened a new pull request #2306:
URL: https://github.com/apache/ozone/pull/2306


   ## What changes were proposed in this pull request?
   
   `StreamingGenerator` is created as part of HDDS-5142, but in the original version the number of threads as forced to be 1.
   
   This test starts a new streaming server in each iteration and streams a pre-generated directory via the Ozone (!) streaming API (old directory is removed, and the new directory will be used as source in the next iteration)
   
   This patch extends the original test with accepting the standard `-t` parameter and use thread specific working directory for the streaming 
   
   ## How was this patch tested?
   
   ```
   ./ozone freon  strmg -n1 --files=1 -t=10
   ```


-- 
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] elek commented on a change in pull request #2306: HDDS-5310. Enhance freon streaming generator to support multiple threads

Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #2306:
URL: https://github.com/apache/ozone/pull/2306#discussion_r650926459



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/StreamingGenerator.java
##########
@@ -76,47 +77,60 @@ public Void call() throws Exception {
     generateBaseData();
 
     timer = getMetrics().timer("streaming");
-    setThreadNo(1);
     runTests(this::copyDir);
 
     return null;
   }
 
-  private void generateBaseData() throws IOException {
-    Path sourceDir = testRoot.resolve("streaming-0");
-    if (Files.exists(sourceDir)) {
-      deleteDirRecursive(sourceDir);
-    }
-    Path subDir = sourceDir.resolve(subdir);
-    Files.createDirectories(subDir);
-    ContentGenerator contentGenerator = new ContentGenerator(fileSize,
-        1024);
-
-    for (int i = 0; i < numberOfFiles; i++) {
-      try (FileOutputStream out = new FileOutputStream(
-          subDir.resolve("file-" + i).toFile())
-      ) {
-        contentGenerator.write(out);
+  private void generateBaseData() {
+    try {
+      Path sourceDir = threadRootDir().resolve("streaming-0");
+      final Path sourceDirParent = sourceDir.getParent();
+      if (sourceDirParent == null) {
+        throw new AssertionError("Empty parrent");

Review comment:
       It couldn't be null normally, but `spotbugs` complains about it. (If any path is `/` path, parent is `null`).
   
   But using `threadRootDir()` instead of using `getParent()` call -- what you suggested -- solves this problem as spotbugs shouldn't complain about it any more.
   
   I am updating this...




-- 
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] elek commented on a change in pull request #2306: HDDS-5310. Enhance freon streaming generator to support multiple threads

Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #2306:
URL: https://github.com/apache/ozone/pull/2306#discussion_r650926676



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/StreamingGenerator.java
##########
@@ -76,47 +77,60 @@ public Void call() throws Exception {
     generateBaseData();

Review comment:
       Yep, thanks. Removing it.




-- 
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] elek commented on a change in pull request #2306: HDDS-5310. Enhance freon streaming generator to support multiple threads

Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #2306:
URL: https://github.com/apache/ozone/pull/2306#discussion_r650934828



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/StreamingGenerator.java
##########
@@ -76,47 +77,60 @@ public Void call() throws Exception {
     generateBaseData();
 
     timer = getMetrics().timer("streaming");
-    setThreadNo(1);
     runTests(this::copyDir);
 
     return null;
   }
 
-  private void generateBaseData() throws IOException {
-    Path sourceDir = testRoot.resolve("streaming-0");
-    if (Files.exists(sourceDir)) {
-      deleteDirRecursive(sourceDir);
-    }
-    Path subDir = sourceDir.resolve(subdir);
-    Files.createDirectories(subDir);
-    ContentGenerator contentGenerator = new ContentGenerator(fileSize,
-        1024);
-
-    for (int i = 0; i < numberOfFiles; i++) {
-      try (FileOutputStream out = new FileOutputStream(
-          subDir.resolve("file-" + i).toFile())
-      ) {
-        contentGenerator.write(out);
+  private void generateBaseData() {
+    try {
+      Path sourceDir = threadRootDir().resolve("streaming-0");
+      final Path sourceDirParent = sourceDir.getParent();
+      if (sourceDirParent == null) {
+        throw new AssertionError("Empty parrent");
       }
+      if (Files.exists(sourceDirParent)) {
+        deleteDirRecursive(sourceDirParent);
+      }
+      Path subDir = sourceDir.resolve(subdir);

Review comment:
       Gooid point. I converted the first one to a constant: `private static final String SUB_DIR_NAME = "dir1";` 




-- 
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] elek commented on pull request #2306: HDDS-5310. Enhance freon streaming generator to support multiple threads

Posted by GitBox <gi...@apache.org>.
elek commented on pull request #2306:
URL: https://github.com/apache/ozone/pull/2306#issuecomment-883155052


   Merging it after the green build. Thanks the review @adoroszlai 


-- 
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: issues-unsubscribe@ozone.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] elek merged pull request #2306: HDDS-5310. Enhance freon streaming generator to support multiple threads

Posted by GitBox <gi...@apache.org>.
elek merged pull request #2306:
URL: https://github.com/apache/ozone/pull/2306


   


-- 
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: issues-unsubscribe@ozone.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] elek commented on pull request #2306: HDDS-5310. Enhance freon streaming generator to support multiple threads

Posted by GitBox <gi...@apache.org>.
elek commented on pull request #2306:
URL: https://github.com/apache/ozone/pull/2306#issuecomment-883155052


   Merging it after the green build. Thanks the review @adoroszlai 


-- 
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: issues-unsubscribe@ozone.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] elek merged pull request #2306: HDDS-5310. Enhance freon streaming generator to support multiple threads

Posted by GitBox <gi...@apache.org>.
elek merged pull request #2306:
URL: https://github.com/apache/ozone/pull/2306


   


-- 
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: issues-unsubscribe@ozone.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] elek merged pull request #2306: HDDS-5310. Enhance freon streaming generator to support multiple threads

Posted by GitBox <gi...@apache.org>.
elek merged pull request #2306:
URL: https://github.com/apache/ozone/pull/2306


   


-- 
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: issues-unsubscribe@ozone.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on a change in pull request #2306: HDDS-5310. Enhance freon streaming generator to support multiple threads

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2306:
URL: https://github.com/apache/ozone/pull/2306#discussion_r649081070



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/StreamingGenerator.java
##########
@@ -76,47 +77,60 @@ public Void call() throws Exception {
     generateBaseData();
 
     timer = getMetrics().timer("streaming");
-    setThreadNo(1);
     runTests(this::copyDir);
 
     return null;
   }
 
-  private void generateBaseData() throws IOException {
-    Path sourceDir = testRoot.resolve("streaming-0");
-    if (Files.exists(sourceDir)) {
-      deleteDirRecursive(sourceDir);
-    }
-    Path subDir = sourceDir.resolve(subdir);
-    Files.createDirectories(subDir);
-    ContentGenerator contentGenerator = new ContentGenerator(fileSize,
-        1024);
-
-    for (int i = 0; i < numberOfFiles; i++) {
-      try (FileOutputStream out = new FileOutputStream(
-          subDir.resolve("file-" + i).toFile())
-      ) {
-        contentGenerator.write(out);
+  private void generateBaseData() {
+    try {
+      Path sourceDir = threadRootDir().resolve("streaming-0");
+      final Path sourceDirParent = sourceDir.getParent();
+      if (sourceDirParent == null) {
+        throw new AssertionError("Empty parrent");

Review comment:
       Doesn't `sourceDir.getParent()` resolve to the same path as `threadRootDir()`?  Also, how can it be `null`?

##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/StreamingGenerator.java
##########
@@ -76,47 +77,60 @@ public Void call() throws Exception {
     generateBaseData();
 
     timer = getMetrics().timer("streaming");
-    setThreadNo(1);
     runTests(this::copyDir);
 
     return null;
   }
 
-  private void generateBaseData() throws IOException {
-    Path sourceDir = testRoot.resolve("streaming-0");
-    if (Files.exists(sourceDir)) {
-      deleteDirRecursive(sourceDir);
-    }
-    Path subDir = sourceDir.resolve(subdir);
-    Files.createDirectories(subDir);
-    ContentGenerator contentGenerator = new ContentGenerator(fileSize,
-        1024);
-
-    for (int i = 0; i < numberOfFiles; i++) {
-      try (FileOutputStream out = new FileOutputStream(
-          subDir.resolve("file-" + i).toFile())
-      ) {
-        contentGenerator.write(out);
+  private void generateBaseData() {
+    try {
+      Path sourceDir = threadRootDir().resolve("streaming-0");
+      final Path sourceDirParent = sourceDir.getParent();
+      if (sourceDirParent == null) {
+        throw new AssertionError("Empty parrent");
       }
+      if (Files.exists(sourceDirParent)) {
+        deleteDirRecursive(sourceDirParent);
+      }
+      Path subDir = sourceDir.resolve(subdir);
+      Files.createDirectories(subDir);
+      ContentGenerator contentGenerator = new ContentGenerator(fileSize,
+          1024);
+
+      for (int i = 0; i < numberOfFiles; i++) {
+        try (FileOutputStream out = new FileOutputStream(
+            subDir.resolve("file-" + i).toFile())
+        ) {
+          contentGenerator.write(out);
+        }
+      }
+    } catch (Exception ex) {
+      throw new RuntimeException(ex);
     }
   }
 
   private void copyDir(long l) {
-    Path sourceDir = testRoot.resolve("streaming-" + l);
-    Path destinationDir = testRoot.resolve("streaming-" + (l + 1));
+    Integer index = counter.get();
+    if (index == null) {
+      generateBaseData();
+      index = 0;
+    }
+    Path sourceDir = threadRootDir().resolve("streaming-" + index);
+    Path destinationDir = threadRootDir().resolve("streaming-" + (index + 1));
+    counter.set(index + 1);
 
+    int port = (int) (1234 + l);

Review comment:
       Extreme number of tests may lead to invalid ports.  Can it be guarded with some kind of modulo arithmetic?

##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/StreamingGenerator.java
##########
@@ -76,47 +77,60 @@ public Void call() throws Exception {
     generateBaseData();

Review comment:
       I guess we no longer need to generate base data in main thread.

##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/StreamingGenerator.java
##########
@@ -76,47 +77,60 @@ public Void call() throws Exception {
     generateBaseData();
 
     timer = getMetrics().timer("streaming");
-    setThreadNo(1);
     runTests(this::copyDir);
 
     return null;
   }
 
-  private void generateBaseData() throws IOException {
-    Path sourceDir = testRoot.resolve("streaming-0");
-    if (Files.exists(sourceDir)) {
-      deleteDirRecursive(sourceDir);
-    }
-    Path subDir = sourceDir.resolve(subdir);
-    Files.createDirectories(subDir);
-    ContentGenerator contentGenerator = new ContentGenerator(fileSize,
-        1024);
-
-    for (int i = 0; i < numberOfFiles; i++) {
-      try (FileOutputStream out = new FileOutputStream(
-          subDir.resolve("file-" + i).toFile())
-      ) {
-        contentGenerator.write(out);
+  private void generateBaseData() {
+    try {
+      Path sourceDir = threadRootDir().resolve("streaming-0");
+      final Path sourceDirParent = sourceDir.getParent();
+      if (sourceDirParent == null) {
+        throw new AssertionError("Empty parrent");
       }
+      if (Files.exists(sourceDirParent)) {
+        deleteDirRecursive(sourceDirParent);
+      }
+      Path subDir = sourceDir.resolve(subdir);

Review comment:
       It's a bit confusing to have two such similar variables (`subdir` vs `subDir`).




-- 
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] elek commented on pull request #2306: HDDS-5310. Enhance freon streaming generator to support multiple threads

Posted by GitBox <gi...@apache.org>.
elek commented on pull request #2306:
URL: https://github.com/apache/ozone/pull/2306#issuecomment-883155052


   Merging it after the green build. Thanks the review @adoroszlai 


-- 
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: issues-unsubscribe@ozone.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] elek commented on a change in pull request #2306: HDDS-5310. Enhance freon streaming generator to support multiple threads

Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #2306:
URL: https://github.com/apache/ozone/pull/2306#discussion_r650935027



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/StreamingGenerator.java
##########
@@ -76,47 +77,60 @@ public Void call() throws Exception {
     generateBaseData();
 
     timer = getMetrics().timer("streaming");
-    setThreadNo(1);
     runTests(this::copyDir);
 
     return null;
   }
 
-  private void generateBaseData() throws IOException {
-    Path sourceDir = testRoot.resolve("streaming-0");
-    if (Files.exists(sourceDir)) {
-      deleteDirRecursive(sourceDir);
-    }
-    Path subDir = sourceDir.resolve(subdir);
-    Files.createDirectories(subDir);
-    ContentGenerator contentGenerator = new ContentGenerator(fileSize,
-        1024);
-
-    for (int i = 0; i < numberOfFiles; i++) {
-      try (FileOutputStream out = new FileOutputStream(
-          subDir.resolve("file-" + i).toFile())
-      ) {
-        contentGenerator.write(out);
+  private void generateBaseData() {
+    try {
+      Path sourceDir = threadRootDir().resolve("streaming-0");
+      final Path sourceDirParent = sourceDir.getParent();
+      if (sourceDirParent == null) {
+        throw new AssertionError("Empty parrent");
       }
+      if (Files.exists(sourceDirParent)) {
+        deleteDirRecursive(sourceDirParent);
+      }
+      Path subDir = sourceDir.resolve(subdir);
+      Files.createDirectories(subDir);
+      ContentGenerator contentGenerator = new ContentGenerator(fileSize,
+          1024);
+
+      for (int i = 0; i < numberOfFiles; i++) {
+        try (FileOutputStream out = new FileOutputStream(
+            subDir.resolve("file-" + i).toFile())
+        ) {
+          contentGenerator.write(out);
+        }
+      }
+    } catch (Exception ex) {
+      throw new RuntimeException(ex);
     }
   }
 
   private void copyDir(long l) {
-    Path sourceDir = testRoot.resolve("streaming-" + l);
-    Path destinationDir = testRoot.resolve("streaming-" + (l + 1));
+    Integer index = counter.get();
+    if (index == null) {
+      generateBaseData();
+      index = 0;
+    }
+    Path sourceDir = threadRootDir().resolve("streaming-" + index);
+    Path destinationDir = threadRootDir().resolve("streaming-" + (index + 1));
+    counter.set(index + 1);
 
+    int port = (int) (1234 + l);

Review comment:
       Sure, added module 64k.




-- 
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org