adoroszlai commented on code in PR #4251:
URL: https://github.com/apache/ozone/pull/4251#discussion_r1098444618


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/BaseFreonGenerator.java:
##########
@@ -484,6 +548,10 @@ public void setThreadNo(int threadNo) {
     this.threadNo = threadNo;
   }
 
+  public long getThreadSequenceId() {
+    return threadSequenceId.get();

Review Comment:
   Please add javadoc comment explaining purpose of this new sequence ID.



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/BaseFreonGenerator.java:
##########
@@ -117,6 +138,11 @@ public class BaseFreonGenerator {
   private ExecutorService executor;
   private ProgressBar progressBar;
 
+  private final ThreadLocal<Long> threadSequenceId = new ThreadLocal<>();
+  private final AtomicLong id = new AtomicLong(0);
+
+  private final AtomicBoolean completion = new AtomicBoolean(false);

Review Comment:
   Nit: please rename to `completed`.



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/BaseFreonGenerator.java:
##########
@@ -92,6 +98,18 @@ public class BaseFreonGenerator {
       defaultValue = "10")
   private int threadNo;
 
+  @Option(names = {"--timebase"},
+      description = "If set, freon will run for the duration of the --runtime"
+          + " specified even if the --number-of-tests operation"
+          + " has been completed.",
+      defaultValue = "false")
+  private boolean timebase;
+
+  @Option(names = {"--runtime"},
+      description = "Tell freon to terminate processing after"
+          + "the specified period of time in seconds.")
+  private long runtime;

Review Comment:
   I would prefer `--min-duration` and `--max-duration` options instead of 
`--runtime` and `--timebase`.  Min and max may be used in any combination (i.e. 
none, either or both of them), the only restriction being `min <= max`.
   
   Also, it would be nice to accept duration in the format we accept in config 
files (see `TimeDurationUtil` for code).



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/BaseFreonGenerator.java:
##########
@@ -266,14 +311,27 @@ public void init() {
         }, 10);
 
     executor = Executors.newFixedThreadPool(threadNo);
-
-    progressBar = new ProgressBar(System.out, testNo, successCounter::get,
-        freonCommand.isInteractive());

Review Comment:
   Please keep the `isInteractive` flag.  Progress bar should not be 
console-based in non-interactive environment.  (Try `docker-compose exec -T scm 
ozone freon ...` for testing.)



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/BaseFreonGenerator.java:
##########
@@ -311,6 +369,12 @@ public void printReport() {
     Consumer<String> print = freonCommand.isInteractive()
         ? System.out::println
         : LOG::info;
+
+    messages.add("\nOption:");
+    for (CommandLine.Model.OptionSpec option : spec.options()) {
+      String name = option.longestName();
+      messages.add(name + "=" + option.getValue());
+    }

Review Comment:
   Can you please print these only for `--verbose` mode?  (Ideally please 
extract this part to a separate task.)



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to