Apache9 commented on a change in pull request #2682:
URL: https://github.com/apache/hbase/pull/2682#discussion_r527491930



##########
File path: 
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java
##########
@@ -1481,6 +1489,31 @@ void onTakedown() throws IOException {
     }
   }
 
+  /*
+  Parent class for all meta tests: MetaWriteTest, MetaRandomReadTest and 
CleanMetaTest
+   */
+  static abstract class MetaTest extends TableTest {
+    protected int keyLength;
+
+    MetaTest(Connection con, TestOptions options, Status status) {
+      super(con, options, status);
+      keyLength = Integer.toString(opts.perClientRunRows).length();
+    }
+
+    @Override
+    void onTakedown() throws IOException {
+      // No clean up
+    }
+
+    /*
+    Generates Lexicographically ascending strings

Review comment:
       Ditto.

##########
File path: 
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java
##########
@@ -1481,6 +1489,31 @@ void onTakedown() throws IOException {
     }
   }
 
+  /*
+  Parent class for all meta tests: MetaWriteTest, MetaRandomReadTest and 
CleanMetaTest

Review comment:
       nits: add a * before the line.

##########
File path: 
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java
##########
@@ -1999,6 +2032,47 @@ protected void testTakedown() throws IOException {
     }
   }
 
+  /*
+  Send random reads against fake regions inserted by MetaWriteTest
+   */
+  static class MetaRandomReadTest extends MetaTest {
+    private Random rd = new Random();
+    private RegionLocator regionLocator;
+
+    MetaRandomReadTest(Connection con, TestOptions options, Status status) {
+      super(con, options, status);
+      LOG.info("call getRegionLocation");
+    }
+
+    @Override
+    void onStartup() throws IOException {
+      super.onStartup();
+      this.regionLocator = connection.getRegionLocator(table.getName());
+    }
+
+    @Override
+    boolean testRow(final int i, final long startTime) throws IOException, 
InterruptedException {
+      if (opts.randomSleep > 0) {
+        Thread.sleep(rd.nextInt(opts.randomSleep));
+      }
+      HRegionLocation hRegionLocation = regionLocator.getRegionLocation(
+        getSplitKey(rd.nextInt(opts.perClientRunRows)), true);
+      LOG.debug("get location for region: " + hRegionLocation);
+      return true;
+    }
+
+    @Override
+    protected int getReportingPeriod() {
+      int period = opts.perClientRunRows / 10;

Review comment:
       Out of interest, we have several pattern of the implementations for this 
method. Some just returns opts.perClientRunRows, some divide by 10, some divide 
by 100. What's the difference here? Why we choose 10 here? Because the random 
read test uses 10? Not a big concern, just asking.

##########
File path: 
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java
##########
@@ -1999,6 +2032,47 @@ protected void testTakedown() throws IOException {
     }
   }
 
+  /*
+  Send random reads against fake regions inserted by MetaWriteTest
+   */
+  static class MetaRandomReadTest extends MetaTest {
+    private Random rd = new Random();

Review comment:
       Since we do not pre set a seed here, please just use ThreadLocalRandom 
instead.




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


Reply via email to