-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/98/#review176
-----------------------------------------------------------


This looks great!  I think we should think more about where to expose this, and 
also think about using more of a get/set API to reduce the method calls and 
make it look more like the other client APIs.


src/main/java/org/apache/hadoop/hbase/client/HConnection.java
<http://review.hbase.org/r/98/#comment841>

    +1 on collapsing with a boolean.  setRegionCachePrefetch(table, enable)?  
Seems self descriptive and with a couple lines of javadoc should be clear.



src/main/java/org/apache/hadoop/hbase/client/HTable.java
<http://review.hbase.org/r/98/#comment840>

    I guess these are static because of how HTables all share a single HCM per 
conf.  The setting of prefetching is set at the HCM level not the HTable level, 
however clients are usually not exposed to HCM and only deal with HTable.
    
    We should probably make it clear in the javadoc for these methods that they 
apply to all HTable instances, though that may be clear from being static.
    
    Maybe since these are more advanced calls, they shouldn't be in HTable?  If 
we provide proper documentation, it should be easy enough for a user to grab 
the HCM and apply the config at that level?



src/main/java/org/apache/hadoop/hbase/client/HTable.java
<http://review.hbase.org/r/98/#comment842>

    Should we also make these methods (if we even leave it in HTable) more like 
setRegionCachePrefetch / getRegionCachePrefetch?  HTable is the core client 
class so the less noise we add the better.  We have other methods in client 
APIs of the get/set form like Scan.setCacheBlocks and Put.setWriteToWAL



src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java
<http://review.hbase.org/r/98/#comment843>

    Hmm, wouldn't that be Math.min then?  If rowLimit = 5 and scanner.caching = 
100, we want to only do 5?


- Jonathan


On 2010-06-09 15:50:59, Mingjie Lai wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/98/
> -----------------------------------------------------------
> 
> (Updated 2010-06-09 15:50:59)
> 
> 
> Review request for hbase, Todd Lipcon and stack.
> 
> 
> Summary
> -------
> 
> HBASE-2468: Improvements to prewarm META cache on clients.
> 
> Changes:
> 1. Add new HTable methods which support region info de/serialation, and 
> region cache prewarm: 
> - void serializeRegionInfo(): clients could perform a large scan for all the 
> meta for the table, serialize the meta to a file. MR job can ship a copy of 
> the meta for the table in the DistributedCache
> - Map<HRegionInfo, HServerAddress> deserializeRegionInfo(): MR job can 
> deserialize the region info from the DistributedCache 
> - prewarmRegionCache(Map<HRegionInfo, HServerAddress> regionMap): MR job can 
> prewarm local region cache by the deserialized region info.
> 
> 2. For each client, each region cache read-miss could trigger read-ahead some 
> number of rows in META. This option could be turned on and off for one 
> particular table. 
> 
> 
> This addresses bug HBASE-2468.
>     http://issues.apache.org/jira/browse/HBASE-2468
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/client/HConnection.java 853164d 
>   src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 
> ed18092 
>   src/main/java/org/apache/hadoop/hbase/client/HTable.java 7ec95cb 
>   src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java d3a0c07 
>   src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java 
> 95e494a 
> 
> Diff: http://review.hbase.org/r/98/diff
> 
> 
> Testing
> -------
> 
> Unit tests passed locally for me. 
> 
> 
> Thanks,
> 
> Mingjie
> 
>

Reply via email to