Mailing-List: contact hbase-dev-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: hbase-dev@hadoop.apache.org
Message-ID: <1837438223.1250036174807.JavaMail.jira@brutus>
Date: Tue, 11 Aug 2009 17:16:14 -0700 (PDT)
From: "Jonathan Gray (JIRA)" <jira@apache.org>
To: hbase-dev@hadoop.apache.org
Subject: [jira] Commented: (HBASE-1759) Ability to specify scanner caching
 on a per-scan basis
In-Reply-To: <496882090.1250014215248.JavaMail.jira@brutus>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/jira/browse/HBASE-1759?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12742133#action_12742133 ] 

Jonathan Gray commented on HBASE-1759:
--------------------------------------

HTable change looks good.

Scan class comment seems like too much detail... Maybe just a tiny blurb and link to the method... then put detailed explanation in the method javadoc.  Also, wording is a little confusing "To set the number of rows that will be fetched when calling next on a scanner if it is not served from memory, execute...", not sure what you mean if it is not served from memory... Can guess but I think it's confusing for a new user to see this information in the top of the Scan javadoc.

How about just, to modify scanner caching for just this scan, use this method... in the method javadoc, put the detailed stuff, and be a little more explicit about what you mean by serving from memory (I guess, you would explain what scanner caching is briefly).

(nitpick, there is unnecessary re-ordering of imports... you can leave it, things should be properly ordered, but as you know, we like our patches as short as possible)

Actual code all looks quite good.  I'm +1 on this for reasons you outlined, with doc changes I described.

Thanks Ken!  :)

> Ability to specify scanner caching on a per-scan basis
> ------------------------------------------------------
>
>                 Key: HBASE-1759
>                 URL: https://issues.apache.org/jira/browse/HBASE-1759
>             Project: Hadoop HBase
>          Issue Type: Improvement
>          Components: client
>    Affects Versions: 0.20.0
>            Reporter: Ken Weiner
>         Attachments: HBASE-1759.patch
>
>
> I think that clients should have the ability to configure the scanner caching setting on a per-scan basis via the org.apache.hadoop.hbase.client.Scan object.  I propose adding a new caching property to the Scan class which would override the HTable.scannerCaching property if set.  This would turn the HTable.scannerCaching property into more of a default setting.
> The code inside ClientScanner (an inner-class implementation of ResultScanner) would look like this:
> {code}
>       // Use the caching from the Scan.  If not set, use the default cache setting for this table.
>       if (this.scan.getCaching() > 0) {
>         this.caching = this.scan.getCaching();
>       } else {
>         this.caching = HTable.this.scannerCaching;
>       }
> {code}
> Note that currently the only option for per-scan scanner caching configuration is to modify the state of HTable for each Scan.  This could lead to confusion when HTables are pooled since the pool would potentially end up with many HTables all configured differently.
> I will attach a patch.  I'm looking forward to hearing your comments.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.