ddanielr commented on code in PR #4671:
URL: https://github.com/apache/accumulo/pull/4671#discussion_r1644575724
##########
core/src/main/java/org/apache/accumulo/core/spi/scan/ConfigurableScanServerSelector.java:
##########
@@ -80,7 +81,28 @@
* specified, the set of scan servers that did not specify a group will be
used. Grouping scan
* servers supports at least two use cases. First groups can be used to
dedicate resources for
* certain scans. Second groups can be used to have different hardware/VM
types for scans, for
- * example could have some scans use expensive high memory VMs and others use
cheaper burstable VMs.
+ * example could have some scans use expensive high memory VMs and others use
cheaper burstable
+ * VMs.</li>
+ * <li><b>timeToWaitForScanServers : </b> When there are no scans servers,
this setting determines
+ * how long to wait for scan servers to load before falling back to tablet
servers is desired.
Review Comment:
```suggestion
* how long to wait for scan servers to become available before falling back
to tablet servers.
```
Match documentation update in #4638
##########
test/src/main/java/org/apache/accumulo/test/ScanServerIT_NoServers.java:
##########
@@ -123,16 +127,55 @@ public void testBatchScan() throws Exception {
}
@Test
- public void testScanOfflineTable() throws Exception {
- try (AccumuloClient client =
Accumulo.newClient().from(getClientProps()).build()) {
+ public void testScanWithNoTserverFallback() throws Exception {
+
+ var clientProps = new Properties();
+ clientProps.putAll(getClientProps());
+ String scanServerSelectorProfiles =
"[{'isDefault':true,'maxBusyTimeout':'5m',"
+ + "'busyTimeoutMultiplier':8, 'scanTypeActivations':[],
'timeToWaitForScanServers':'120s',"
+ + "'attemptPlans':[{'servers':'3', 'busyTimeout':'1s'}]}]";
+ clientProps.put("scan.server.selector.impl",
ConfigurableScanServerSelector.class.getName());
+ clientProps.put("scan.server.selector.opts.profiles",
+ scanServerSelectorProfiles.replace("'", "\""));
+
+ try (AccumuloClient client =
Accumulo.newClient().from(clientProps).build()) {
String tableName = getUniqueNames(1)[0];
createTableAndIngest(client, tableName, null, 10, 10, "colf");
client.tableOperations().offline(tableName, true);
- assertThrows(TableOfflineException.class, () -> {
+ assertThrows(TimedOutException.class, () -> {
Review Comment:
A `TableOfflineException` is being thrown instead of a `TimedOutException`
Most likely due to this:
https://github.com/apache/accumulo/blob/3441e148419081266203b32dfcc3a5361cc8baa2/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java#L800
The test passes successfully if the
`client.tableOperations().offline(tableName, true);` operation is removed.
##########
core/src/main/java/org/apache/accumulo/core/spi/scan/ConfigurableScanServerSelector.java:
##########
@@ -80,7 +81,28 @@
* specified, the set of scan servers that did not specify a group will be
used. Grouping scan
* servers supports at least two use cases. First groups can be used to
dedicate resources for
* certain scans. Second groups can be used to have different hardware/VM
types for scans, for
- * example could have some scans use expensive high memory VMs and others use
cheaper burstable VMs.
+ * example could have some scans use expensive high memory VMs and others use
cheaper burstable
+ * VMs.</li>
+ * <li><b>timeToWaitForScanServers : </b> When there are no scans servers,
this setting determines
+ * how long to wait for scan servers to load before falling back to tablet
servers is desired.
+ * Falling back to tablet servers may cause tablets to be loaded that are not
currently loaded. When
+ * this setting is given a wait time and there are no scan servers, it will
wait for scan servers to
+ * be available. This setting avoids loading tablets on tablet servers when
scans servers are
+ * temporarily unavailable which could be caused by normal cluster activity.
You can specify the
+ * wait time using different units to precisely control the wait duration. The
supported units are:
+ * <ul>
+ * <li>"d" for days</li>
+ * <li>"h" for hours</li>
+ * <li>"m" for minutes</li>
+ * <li>"s" for seconds</li>
+ * <li>"ms" for milliseconds</li>
+ * </ul>
+ * If duration is not specified this setting defaults to 0s, and will disable
the wait for scan
+ * servers. When set to a large value, the selector will effectively wait for
scan servers to become
+ * available before falling back to tablet servers. To ensure the selector
never falls back to
+ * scanning tablet servers an unrealistic wait time can be set. For instance
10000d should be
+ * sufficient. Setting Waiting for scan servers is done via
Review Comment:
```suggestion
* servers and will fall back to tablet servers immediately. When set to a
large value, the selector
* will effectively wait for scan servers to become available before falling
back to tablet servers.
* To ensure the selector never falls back to scanning tablet servers an
unrealistic wait time can
* be set. For instance 10000d should be sufficient. Setting Waiting for
scan servers is done via
```
Match documentation updates in #4638
--
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]
