Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.94.0
    • Fix Version/s: 0.94.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      See HBASE-1935 for motivation.
      ClientScanner should be able to exist outside of HTable.
      While we're at it, we can also add an abstract client scanner to easy development of new client side scanners (such as parallel scanners, or per region scanners).

      1. 4439.txt
        30 kB
        Lars Hofhansl
      2. 4439-v1.txt
        32 kB
        Lars Hofhansl
      3. 4439-v2.txt
        37 kB
        Lars Hofhansl

        Activity

        Hide
        Lars Hofhansl added a comment -

        1st cut. Very simple change, mostly just moves some code around.

        • simply moves HTable.ClientScanner to its own toplevel class. ClientScanner is now useable without an instance of an HTable.
        • HTable.getScanner(scan) now clones the scan. Even previously the scan object was actually modified inside the ClientScanner.
        • Some config options (maxScannerResultSize, scannerTimeout)
        • deprecates HTable. {get|set}

          ScannerCaching, so that scannerCaching can also be removed from HTable. Caching should be set through the scan object or the cluster wide config option instead.

        Show
        Lars Hofhansl added a comment - 1st cut. Very simple change, mostly just moves some code around. simply moves HTable.ClientScanner to its own toplevel class. ClientScanner is now useable without an instance of an HTable. HTable.getScanner(scan) now clones the scan. Even previously the scan object was actually modified inside the ClientScanner. Some config options (maxScannerResultSize, scannerTimeout) deprecates HTable. {get|set} ScannerCaching, so that scannerCaching can also be removed from HTable. Caching should be set through the scan object or the cluster wide config option instead.
        Hide
        Lars Hofhansl added a comment -

        Hit submit too early...

        • Some config options (maxScannerResultSize, scannerTimeout) are moved from HTable to ClientScanner.
        • ClientScanner uses static logger.

        Could consider abstracting useful parts in a helper class if we foresee that writing new client scanners would be a common client side task.

        Show
        Lars Hofhansl added a comment - Hit submit too early... Some config options (maxScannerResultSize, scannerTimeout) are moved from HTable to ClientScanner. ClientScanner uses static logger. Could consider abstracting useful parts in a helper class if we foresee that writing new client scanners would be a common client side task.
        Hide
        stack added a comment -

        HTable.getScanner(scan) now clones the scan. Even previously the scan object was actually modified inside the ClientScanner.

        See HBASE-1774 If we commit this, can we close HBASE-1774?

        License missing from ClientScanner.java

        I'm +1 on committing this to TRUNK. You want to do any more in here?

        Show
        stack added a comment - HTable.getScanner(scan) now clones the scan. Even previously the scan object was actually modified inside the ClientScanner. See HBASE-1774 If we commit this, can we close HBASE-1774 ? License missing from ClientScanner.java I'm +1 on committing this to TRUNK. You want to do any more in here?
        Hide
        Lars Hofhansl added a comment -

        I think we can close HBASE_1774. I'll add the license (won't have time until this evening, though).
        Re doing more: I think we can leave at this for now. Once we have more cases for client implemented scanners, there might be some more refactoring to do, to make that easier.

        Show
        Lars Hofhansl added a comment - I think we can close HBASE_1774. I'll add the license (won't have time until this evening, though). Re doing more: I think we can leave at this for now. Once we have more cases for client implemented scanners, there might be some more refactoring to do, to make that easier.
        Hide
        Lars Hofhansl added a comment -

        Hmm... The new ClientScanner still modifies the passed Scan, so somebody using ClientScanner directly would still get Scan modified. In order to avoid this, Scan would have to be copied twice (since HTable also needs to copy it, because it sets its caching on it, at least until we can remove

        {get|set}

        ScannerCaching from HTable).

        Show
        Lars Hofhansl added a comment - Hmm... The new ClientScanner still modifies the passed Scan, so somebody using ClientScanner directly would still get Scan modified. In order to avoid this, Scan would have to be copied twice (since HTable also needs to copy it, because it sets its caching on it, at least until we can remove {get|set} ScannerCaching from HTable).
        Hide
        Lars Hofhansl added a comment -

        ClientScanner also copies the passed Scan now. Not pretty, but since this is going to do at least one roundtrip to HBase it should not be a problem.

        iterator() and next(int) moved to an abstract helper class (that one makes not assumption about transport, RPC, etc).

        Show
        Lars Hofhansl added a comment - ClientScanner also copies the passed Scan now. Not pretty, but since this is going to do at least one roundtrip to HBase it should not be a problem. iterator() and next(int) moved to an abstract helper class (that one makes not assumption about transport, RPC, etc).
        Hide
        Lars Hofhansl added a comment -

        I'm still interesting in committing this. I'll rebase to trunk. I'll remove the extra copy of the Scan in ClientScanner (but add it as a caveat in the javadoc instead). HTable still makes a copy of the Scan (so we can mark HBASE-1774 as a dup of this).

        Show
        Lars Hofhansl added a comment - I'm still interesting in committing this. I'll rebase to trunk. I'll remove the extra copy of the Scan in ClientScanner (but add it as a caveat in the javadoc instead). HTable still makes a copy of the Scan (so we can mark HBASE-1774 as a dup of this).
        Hide
        Lars Hofhansl added a comment -

        See my comment in HBASE-1774. Turns out as it stands Scan objects cannot be copied as the ScanMetrics are collected into the Scan object.

        Show
        Lars Hofhansl added a comment - See my comment in HBASE-1774 . Turns out as it stands Scan objects cannot be copied as the ScanMetrics are collected into the Scan object.
        Hide
        Lars Hofhansl added a comment -

        Rebased patch.
        Depreated some HTable methods, and added prominent notes to the JavaDocs that the passed Scan object will be modified during scanning.

        Show
        Lars Hofhansl added a comment - Rebased patch. Depreated some HTable methods, and added prominent notes to the JavaDocs that the passed Scan object will be modified during scanning.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12508335/4439-v2.txt
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        -1 javadoc. The javadoc tool appears to have generated -152 warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        -1 findbugs. The patch appears to introduce 77 new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in .

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/577//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/577//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/577//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12508335/4439-v2.txt against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 javadoc. The javadoc tool appears to have generated -152 warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 77 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/577//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/577//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/577//console This message is automatically generated.
        Hide
        stack added a comment -

        +1

        You might change the client package-info example code on commit so it goes via this new preferred route. I'm referring to this stuff: http://hbase.apache.org/apidocs/org/apache/hadoop/hbase/client/package-summary.html#client_example

        Show
        stack added a comment - +1 You might change the client package-info example code on commit so it goes via this new preferred route. I'm referring to this stuff: http://hbase.apache.org/apidocs/org/apache/hadoop/hbase/client/package-summary.html#client_example
        Hide
        Lars Hofhansl added a comment -

        I think the preferred/default route would still be to go through HTable.getScanner(...). So for standard clients nothing would change.
        More advanced clients now have an example of how to do a stand alone client side scanner.

        In the end this mostly a refactoring to make the cleaner.

        Show
        Lars Hofhansl added a comment - I think the preferred/default route would still be to go through HTable.getScanner(...). So for standard clients nothing would change. More advanced clients now have an example of how to do a stand alone client side scanner. In the end this mostly a refactoring to make the cleaner.
        Hide
        stack added a comment -

        @Lars Ok. +1 again (and disregard my update of example code petition)

        Show
        stack added a comment - @Lars Ok. +1 again (and disregard my update of example code petition)
        Hide
        Lars Hofhansl added a comment -

        Committed to trunk

        Show
        Lars Hofhansl added a comment - Committed to trunk
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK-security #42 (See https://builds.apache.org/job/HBase-TRUNK-security/42/)
        HBASE-4439 Move ClientScanner out of HTable (Lars H)

        larsh :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/AbstractClientScanner.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK-security #42 (See https://builds.apache.org/job/HBase-TRUNK-security/42/ ) HBASE-4439 Move ClientScanner out of HTable (Lars H) larsh : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/AbstractClientScanner.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #2570 (See https://builds.apache.org/job/HBase-TRUNK/2570/)
        HBASE-4439 Move ClientScanner out of HTable (Lars H)

        larsh :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/AbstractClientScanner.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK #2570 (See https://builds.apache.org/job/HBase-TRUNK/2570/ ) HBASE-4439 Move ClientScanner out of HTable (Lars H) larsh : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/AbstractClientScanner.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java

          People

          • Assignee:
            Lars Hofhansl
            Reporter:
            Lars Hofhansl
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development