hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jonathan Gray" <jg...@apache.org>
Subject Re: Review Request: HBASE-2468: Improvements to prewarm META cache on clients.
Date Thu, 10 Jun 2010 15:53:41 GMT

-----------------------------------------------------------
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
> 
>


Mime
View raw message