dlmarion commented on code in PR #3172:
URL: https://github.com/apache/accumulo/pull/3172#discussion_r1086559302
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchReader.java:
##########
@@ -116,6 +116,6 @@ public Iterator<Entry<Key,Value>> iterator() {
}
return new TabletServerBatchReaderIterator(context, tableId, tableName,
authorizations, ranges,
- numThreads, queryThreadPool, this, timeOut);
+ numThreads, queryThreadPool, this, retryTimeout);
Review Comment:
The BatchScanner interface overrides the ScannerBase interface method
`setTimeout` and has a different definition for it. I'm wondering if it would
be better to go one step further here and change ScannerBase. For example:
```
diff --git
a/core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java
b/core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java
index 5270c45221..a2f9a22c46 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java
@@ -208,17 +208,47 @@ public interface ScannerBase extends
Iterable<Entry<Key,Value>>, AutoCloseable {
* @param timeOut the length of the timeout
* @param timeUnit the units of the timeout
* @since 1.5.0
+ * @deprecated - replaced by {@link #setRetryTimeout(long, TimeUnit)}
*/
- void setTimeout(long timeOut, TimeUnit timeUnit);
+ @Deprecated
+ default void setTimeout(long timeOut, TimeUnit timeUnit) {
+ setRetryTimeout(timeOut, timeUnit);
+ }
/**
* Returns the setting for how long a scanner will automatically retry
when a failure occurs.
*
* @return the timeout configured for this scanner
* @since 1.5.0
+ * @deprecated - replaced by {@link #getRetryTimeout(TimeUnit)}
*/
- long getTimeout(TimeUnit timeUnit);
+ @Deprecated
+ default long getTimeout(TimeUnit timeUnit) {
+ return getRetryTimeout(timeUnit);
+ }
+ /**
+ * This setting determines how long a scanner will automatically retry
when a failure occurs. By
+ * default, a scanner will retry forever.
+ *
+ * <p>
+ * Setting the timeout to zero (with any time unit) or {@link
Long#MAX_VALUE} (with
+ * {@link TimeUnit#MILLISECONDS}) means no timeout.
+ *
+ * @param timeOut the length of the timeout
+ * @param timeUnit the units of the timeout
+ * @since 2.1.1
+ */
+ void setRetryTimeout(long retryTimeout, TimeUnit timeUnit);
+
+ /**
+ * Returns the setting for how long a scanner will automatically retry
when a failure occurs.
+ *
+ * @return the timeout configured for this scanner
+ * @since 2.1.1
+ */
+ long getRetryTimeout(TimeUnit timeUnit);
+
/**
* Closes any underlying connections on the scanner. This may invalidate
any iterators derived
* from the Scanner, causing them to throw exceptions.
```
You would make the same changes that you have here already, changing the
ScannerOptions variable from `timeOut` to `retryTimeout` and renaming the
ScannerOptions set/getTimeout method. However, TabletServerBatchReader would
need it's own `setTimeout` implementation and `timeout` variable for its
different use case.
--
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]
