saintstack commented on a change in pull request #2644:
URL: https://github.com/apache/hbase/pull/2644#discussion_r523303251



##########
File path: 
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java
##########
@@ -1481,6 +1489,28 @@ void onTakedown() throws IOException {
     }
   }
 
+  static abstract class MetaTest extends TableTest {

Review comment:
       No harm in a class comment?

##########
File path: 
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java
##########
@@ -187,11 +192,12 @@
       "Run sequential read test");
     addCommandDescriptor(SequentialWriteTest.class, "sequentialWrite",
       "Run sequential write test");
-    addCommandDescriptor(ScanTest.class, "scan",
-      "Run scan test (read every row)");
+    addCommandDescriptor(MetaWriteTest.class, "metaWrite",
+      "Run addRegion test to populate meta table; used with 1 thread");

Review comment:
       Do we want to tie this to the new cleanMeta? i.e. in the help text here 
say cleanMeta cleans up the writes done by this test run?

##########
File path: 
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java
##########
@@ -2276,7 +2371,29 @@ boolean testRow(final int i, final long startTime) 
throws IOException {
       return true;
     }
   }
+  static class MetaWriteTest extends MetaTest {

Review comment:
       Add empty line above and class comment.

##########
File path: 
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java
##########
@@ -2188,6 +2257,32 @@ boolean testRow(final int i, final long startTime) 
throws IOException {
     }
   }
 
+  static class CleanMetaTest extends MetaTest {
+    CleanMetaTest(Connection con, TestOptions options, Status status) {
+      super(con, options, status);
+    }
+
+    @Override
+    boolean testRow(final int i, final long startTime) throws IOException {
+      try {
+        RegionInfo regionInfo = connection.getRegionLocator(table.getName())
+          .getRegionLocation(getSplitKey(i), false).getRegion();
+        if (LOG.isDebugEnabled()) {

Review comment:
       No need of test for isdebug if parameterized logging.

##########
File path: 
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java
##########
@@ -1998,6 +2028,45 @@ protected void testTakedown() throws IOException {
       super.testTakedown();
     }
   }
+  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);
+      if (LOG.isDebugEnabled()) {

Review comment:
       No need of the if debug test... use parameterized logging... "Get 
location for {}", hRegionLocation.... no need of 'region' in log message.

##########
File path: 
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java
##########
@@ -2188,6 +2257,32 @@ boolean testRow(final int i, final long startTime) 
throws IOException {
     }
   }
 
+  static class CleanMetaTest extends MetaTest {

Review comment:
       No harm in class comment on what this does.

##########
File path: 
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java
##########
@@ -1998,6 +2028,45 @@ protected void testTakedown() throws IOException {
       super.testTakedown();
     }
   }
+  static class MetaRandomReadTest extends MetaTest {

Review comment:
       Add empty line above? Class comment on what this does?




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