EdColeman commented on PR #3166: URL: https://github.com/apache/accumulo/pull/3166#issuecomment-1400652298
@keith-turner Basically there are two timeouts in IsolatedScanner - one is inherited by extending ScannerOptions and the other is local to IsolatedScanner (now named iteratorTimeout in this PR) and referenced below as the "local" value. The scannerTimeout in Scanner options is not referenced in the IsolatedScanner code. I was unsure if the local one was necessary or if the code could delegate to ScannerOptions. Before, the local IsolatedScanner timeout was "timeout" https://github.com/apache/accumulo/blob/459a5c0d5ef97339837a75b61af817f77fc65b42/core/src/main/java/org/apache/accumulo/core/client/IsolatedScanner.java#L52 In ScannerOptions, the parameter was called "timeOut" (note capital O) https://github.com/apache/accumulo/blob/459a5c0d5ef97339837a75b61af817f77fc65b42/core/src/main/java/org/apache/accumulo/core/clientImpl/ScannerOptions.java#L56 When the iterator is created in newIterator, the scanner is set to the "locally declared value" https://github.com/apache/accumulo/blob/0ef06afbfcea0908d3b19e1d8ae9fc1cc60a2e77/core/src/main/java/org/apache/accumulo/core/client/IsolatedScanner.java#L124 In the buffered reader the constructor, it sets the "local" value. https://github.com/apache/accumulo/blob/0ef06afbfcea0908d3b19e1d8ae9fc1cc60a2e77/core/src/main/java/org/apache/accumulo/core/client/IsolatedScanner.java#L137 I was unsure if "timeout" was distinctly different than "timeOut" or if it was an oversight. I thought about deleting the local value and delegating to the scannerTimeout (timeOut) in ScannerOptions, but it was not clear if this would be okay or if the local value was specifically created / used for some purpose that was not clear to me. It's fine if the local value is needed / stays - but a short description as to why may help the next person that comes along and has the same question. The other option is to delegate to the ScannerOptions scannerTimeout (timeOut) and remove the "local" value. Currently, this PR is keeping both to preserve the current functionality. -- 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]
