You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2021/09/30 17:16:14 UTC
[GitHub] [accumulo-testing] DomGarguilo opened a new pull request #163: Fix ScanExecutorPT exceptions
DomGarguilo opened a new pull request #163:
URL: https://github.com/apache/accumulo-testing/pull/163
Fixes #162
* Replace old hint method with new scan_type method of selecting executors
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo-testing] DomGarguilo commented on pull request #163: Fix ScanExecutorPT exceptions
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on pull request #163:
URL: https://github.com/apache/accumulo-testing/pull/163#issuecomment-931514294
I'm not certain that these changes are preserving the exact functionality of the old code. Also, there are a few minor changes that I will include in this PR once all fix-related changes are deemed correct as to not clutter the diff.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo-testing] DomGarguilo merged pull request #163: Fix ScanExecutorPT exceptions
Posted by GitBox <gi...@apache.org>.
DomGarguilo merged pull request #163:
URL: https://github.com/apache/accumulo-testing/pull/163
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo-testing] keith-turner commented on a change in pull request #163: Fix ScanExecutorPT exceptions
Posted by GitBox <gi...@apache.org>.
keith-turner commented on a change in pull request #163:
URL: https://github.com/apache/accumulo-testing/pull/163#discussion_r720304767
##########
File path: src/main/java/org/apache/accumulo/testing/performance/tests/ScanExecutorPT.java
##########
@@ -89,8 +95,9 @@ public Report runTest(Environment env) throws Exception {
String tableName = "scept";
Map<String,String> props = new HashMap<>();
- props.put(Property.TABLE_SCAN_DISPATCHER_OPTS.getKey() + "executor", "se1");
- props.put(Property.TABLE_SCAN_DISPATCHER_OPTS.getKey() + "heed_hints", "true");
+ props.put(Property.TABLE_SCAN_DISPATCHER_OPTS.getKey() + "executor.se1p1", "se1");
+ props.put(Property.TABLE_SCAN_DISPATCHER_OPTS.getKey() + "executor.se1p2", "se1");
+ props.put(Property.TABLE_SCAN_DISPATCHER_OPTS.getKey() + "executor.se2p1", "se2");
Review comment:
Yeah you do need two scan types. This per table dispatcher does not care about prio, it just dispatches scans to the executor se1 or se2. The executor se1 has further config that sets the prio based on scan type, which is where the two diff scan types matter. So we have it dispatch everything to executor se1 unless the scan type is dedicated in which case it dispatches to se2.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo-testing] DomGarguilo commented on a change in pull request #163: Fix ScanExecutorPT exceptions
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on a change in pull request #163:
URL: https://github.com/apache/accumulo-testing/pull/163#discussion_r720300297
##########
File path: src/main/java/org/apache/accumulo/testing/performance/tests/ScanExecutorPT.java
##########
@@ -89,8 +95,9 @@ public Report runTest(Environment env) throws Exception {
String tableName = "scept";
Map<String,String> props = new HashMap<>();
- props.put(Property.TABLE_SCAN_DISPATCHER_OPTS.getKey() + "executor", "se1");
- props.put(Property.TABLE_SCAN_DISPATCHER_OPTS.getKey() + "heed_hints", "true");
+ props.put(Property.TABLE_SCAN_DISPATCHER_OPTS.getKey() + "executor.se1p1", "se1");
+ props.put(Property.TABLE_SCAN_DISPATCHER_OPTS.getKey() + "executor.se1p2", "se1");
+ props.put(Property.TABLE_SCAN_DISPATCHER_OPTS.getKey() + "executor.se2p1", "se2");
Review comment:
In the test, se1 is used at two different priorities though so I thought we would need to define two scan types: se1 with priority 1 and the other se1 with priority 2. @keith-turner
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo-testing] keith-turner commented on a change in pull request #163: Fix ScanExecutorPT exceptions
Posted by GitBox <gi...@apache.org>.
keith-turner commented on a change in pull request #163:
URL: https://github.com/apache/accumulo-testing/pull/163#discussion_r719829168
##########
File path: src/main/java/org/apache/accumulo/testing/performance/tests/ScanExecutorPT.java
##########
@@ -89,8 +95,9 @@ public Report runTest(Environment env) throws Exception {
String tableName = "scept";
Map<String,String> props = new HashMap<>();
- props.put(Property.TABLE_SCAN_DISPATCHER_OPTS.getKey() + "executor", "se1");
- props.put(Property.TABLE_SCAN_DISPATCHER_OPTS.getKey() + "heed_hints", "true");
+ props.put(Property.TABLE_SCAN_DISPATCHER_OPTS.getKey() + "executor.se1p1", "se1");
+ props.put(Property.TABLE_SCAN_DISPATCHER_OPTS.getKey() + "executor.se1p2", "se1");
+ props.put(Property.TABLE_SCAN_DISPATCHER_OPTS.getKey() + "executor.se2p1", "se2");
Review comment:
If we just set the executor property then that will be the default for all scan types, so do not need to set it twice. When scan_type equals dedicated, it will override it to se2.
```suggestion
props.put(Property.TABLE_SCAN_DISPATCHER_OPTS.getKey() + "executor", "se1");
props.put(Property.TABLE_SCAN_DISPATCHER_OPTS.getKey() + "executor.dedicated", "se2");
```
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo-testing] DomGarguilo commented on pull request #163: Fix ScanExecutorPT exceptions
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on pull request #163:
URL: https://github.com/apache/accumulo-testing/pull/163#issuecomment-931515903
@keith-turner, it looks like you implemented these changes in https://github.com/apache/accumulo/pull/972 so I was wondering if you might have some thoughts regarding these changes.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo-testing] keith-turner commented on a change in pull request #163: Fix ScanExecutorPT exceptions
Posted by GitBox <gi...@apache.org>.
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo-testing] DomGarguilo commented on pull request #163: Fix ScanExecutorPT exceptions
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on pull request #163:
URL: https://github.com/apache/accumulo-testing/pull/163#issuecomment-932379637
> Changes look good. The test completes as expected. I did notice a few small corrections to make with other parts of the test but I will wait until you push your other changes up just in case they are already handled.
@Manno15, I included the other changes I had in mind in da3635d. Let me know if you have some more improvements.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org