keith-turner commented on a change in pull request #163:
URL: https://github.com/apache/accumulo-testing/pull/163#discussion_r719817329



##########
File path: 
src/main/java/org/apache/accumulo/testing/performance/tests/ScanExecutorPT.java
##########
@@ -75,10 +74,17 @@ public SystemConfiguration getSystemConfig() {
         SCAN_EXECUTOR_THREADS);
     siteCfg.put(Property.TSERV_SCAN_EXECUTORS_PREFIX.getKey() + 
"se1.prioritizer",
         SCAN_PRIORITIZER);
+    siteCfg.put(
+        Property.TSERV_SCAN_EXECUTORS_PREFIX.getKey() + 
"se1.prioritizer.opts.priority.se1p1", "1");
+    siteCfg.put(
+        Property.TSERV_SCAN_EXECUTORS_PREFIX.getKey() + 
"se1.prioritizer.opts.priority.se1p2", "2");
+

Review comment:
       ```suggestion
       siteCfg.put(
           Property.TSERV_SCAN_EXECUTORS_PREFIX.getKey() + 
"se1.prioritizer.opts.priority.short", "1");
       siteCfg.put(
           Property.TSERV_SCAN_EXECUTORS_PREFIX.getKey() + 
"se1.prioritizer.opts.priority.long", "2");
   
   ```

##########
File path: 
src/main/java/org/apache/accumulo/testing/performance/tests/ScanExecutorPT.java
##########
@@ -75,10 +74,17 @@ public SystemConfiguration getSystemConfig() {
         SCAN_EXECUTOR_THREADS);
     siteCfg.put(Property.TSERV_SCAN_EXECUTORS_PREFIX.getKey() + 
"se1.prioritizer",
         SCAN_PRIORITIZER);
+    siteCfg.put(
+        Property.TSERV_SCAN_EXECUTORS_PREFIX.getKey() + 
"se1.prioritizer.opts.priority.se1p1", "1");
+    siteCfg.put(
+        Property.TSERV_SCAN_EXECUTORS_PREFIX.getKey() + 
"se1.prioritizer.opts.priority.se1p2", "2");
+
     siteCfg.put(Property.TSERV_SCAN_EXECUTORS_PREFIX.getKey() + "se2.threads",
         SCAN_EXECUTOR_THREADS);
     siteCfg.put(Property.TSERV_SCAN_EXECUTORS_PREFIX.getKey() + 
"se2.prioritizer",
         SCAN_PRIORITIZER);
+    siteCfg.put(
+        Property.TSERV_SCAN_EXECUTORS_PREFIX.getKey() + 
"se2.prioritizer.opts.priority.se2p1", "1");

Review comment:
       We can omit this if everything on the se2 pool will have the same prio, 
then it does not matter what it is. The default would be fine.
   
   ```suggestion
   ```

##########
File path: 
src/main/java/org/apache/accumulo/testing/performance/tests/ScanExecutorPT.java
##########
@@ -178,11 +185,10 @@ private long scan(String tableName, AccumuloClient c, 
AtomicBoolean stop,
     return count;
   }
 
-  private LongSummaryStatistics runShortScans(Environment env, String 
tableName, int numScans)
-      throws InterruptedException, ExecutionException {
+  private LongSummaryStatistics runShortScans(Environment env, String 
tableName, int numScans) {
 
-    Map<String,String> execHints = ImmutableMap.of("executor", "se2");
-    Map<String,String> prioHints = ImmutableMap.of("priority", "1");
+    Map<String,String> execHints = ImmutableMap.of("scan_type", "se2p1");
+    Map<String,String> prioHints = ImmutableMap.of("scan_type", "se1p1");

Review comment:
       Could change the map names and use more descriptive name for the scan 
types.
   
   ```suggestion
       Map<String,String> dHints = ImmutableMap.of("scan_type", "dedicated");
       Map<String,String> sHints = ImmutableMap.of("scan_type", "short");
   ```

##########
File path: 
src/main/java/org/apache/accumulo/testing/performance/tests/ScanExecutorPT.java
##########
@@ -201,7 +207,7 @@ private LongSummaryStatistics runShortScans(Environment 
env, String tableName, i
   }
 
   private TestExecutor<Long> startLongScans(Environment env, String tableName, 
AtomicBoolean stop) {
-    Map<String,String> hints = ImmutableMap.of("priority", "2");
+    Map<String,String> hints = Map.of("scan_type", "se1p2");

Review comment:
       ```suggestion
       Map<String,String> hints = Map.of("scan_type", "long");
   ```




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


Reply via email to