[
https://issues.apache.org/jira/browse/HBASE-17583?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15863068#comment-15863068
]
Ted Yu commented on HBASE-17583:
--------------------------------
For ClientScanner :
{code}
+ // will start next scan from the startKey of the currentRegion. Return false
if we have reached
+ // the stop row.
+ protected abstract boolean setNewStartKey();
{code}
formulate the description of @return using javadoc.
{code}
+ // return false if we should terminate the scan
+ // protected only because TestClientScanner need to override this method.
+ @VisibleForTesting
+ protected boolean moveToNextRegion() {
{code}
formulate the description of @return using javadoc.
{code}
+ // not a big deal continue
+ LOG.warn("close scanner for " + currentRegion + " failed", e);
{code}
If not a big deal, log at DEBUG level.
{code}
private Result[] call(ScannerCallableWithReplicas callable,
RpcRetryingCaller<Result[]> caller,
- int scannerTimeout) throws IOException {
+ int scannerTimeout, boolean updateCurrentRegion) throws IOException {
{code}
Add javadoc for parameter updateCurrentRegion.
{code}
private boolean scanExhausted(Result[] values) {
- // This means the server tells us the whole scan operation is done.
Usually decided by filter or
- // limit.
- return values == null || callable.moreResultsForScan() == MoreResults.NO;
+ return callable.moreResultsForScan() == MoreResults.NO;
{code}
Now the method becomes a single comparison. It seems there is no need for this
method.
{code}
+ // If lastResult is partial then include it, otherwise exlude it.
+ scan.withStartRow(lastResult.getRow(), lastResult.isPartial() ||
scan.getBatch() > 0);
{code}
Typo: exlude
{code}
+ // TODO: Why wrap this in a DNRIOE when it already is a DNRIOE?
+ throw new DoNotRetryIOException(
+ "Failed after retry of OutOfOrderScannerNextException: was there a
rpc timeout?", e);
{code}
Drop the wrapping ?
{code}
- // We need to reset it if it's a new callable that was created with a
countdown in nextScanner
- callable.setCaching(this.caching);
{code}
Why is the above dropped ?
{code}
+public class ClientSimpleScanner extends ClientScanner {
{code}
ClientSimpleScanner -> SimpleClientScanner
{code}
+ // we have not implemented locate to next row for sync client yet, so
here we change the
+ // inclusive of start row to true.
+ scan.withStartRow(createClosestRowAfter(scan.getStartRow()), true);
{code}
When would locating next row be implemented.
{code}
+ static boolean isEmptyStartRow(byte[] row) {
+ return Bytes.equals(row, EMPTY_START_ROW);
+ }
+
+ static boolean isEmptyStopRow(byte[] row) {
+ return Bytes.equals(row, EMPTY_END_ROW);
{code}
Since the same byte array is being compared with, only one method suffices:
isEmptyRow().
The patch is sizable.
Please put it on review board.
> Add inclusive/exclusive support for startRow and endRow of scan for sync
> client
> -------------------------------------------------------------------------------
>
> Key: HBASE-17583
> URL: https://issues.apache.org/jira/browse/HBASE-17583
> Project: HBase
> Issue Type: Sub-task
> Components: Client, scan
> Affects Versions: 2.0.0, 1.4.0
> Reporter: Duo Zhang
> Assignee: Duo Zhang
> Fix For: 2.0.0, 1.4.0
>
> Attachments: HBASE-17583-branch-1.patch, HBASE-17583.patch,
> HBASE-17583-v1.patch, HBASE-17583-v2.patch
>
>
> Implement the same feature of HBASE-17320 for sync client.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
