cshannon commented on code in PR #4866:
URL: https://github.com/apache/accumulo/pull/4866#discussion_r1756624526
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ScannerImpl.java:
##########
@@ -163,7 +163,7 @@ public synchronized int getBatchSize() {
public synchronized Iterator<Entry<Key,Value>> iterator() {
ensureOpen();
ScannerIterator iter = new ScannerIterator(context, tableId,
authorizations, range, size,
- getTimeout(SECONDS), this, isolated, readaheadThreshold, new
Reporter());
+ Duration.ofMillis(retryTimeout), this, isolated, readaheadThreshold,
new Reporter());
Review Comment:
Good point on using getTimeout() method, I'll change that.
In regards to converting getTimeout() to a Duration...right that is the
exact problem I mentioned in my PR, it cascades very quickly and starts to
impact several other classes as the timeouts are passed around so changing it
to a Duration means changing a lot of other stuff. Also, for example, there's
also getBatchTimeout() and I don't think it makes sense to change one and not
the other.
So that's why I was thinking it would be better to minimize the changes in
2.1 at this point. We could make it a follow on for 3.1 and main. However, if
it you think it's worthwhile for 2.1 I can still make the changes in 2.1 (even
if it's another PR)
--
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]
