hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "jiraposter@reviews.apache.org (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-4027) Enable direct byte buffers LruBlockCache
Date Sat, 30 Jul 2011 00:57:11 GMT

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

jiraposter@reviews.apache.org commented on HBASE-4027:
------------------------------------------------------



bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > conf/hbase-env.sh, lines 44-45
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28125#file28125line44>
bq.  >
bq.  >     Is MaxDirectMemorySize determinable on the running jvm?  Could we make the offheapcachesize
config as a percentage of the direct memory size like we have for memstore/blockcache today?
 (default of 0.95 or something would make it so it never really has to be set for most cases...
and i'm not sure what exactly "a bit above the off heap cache size" is)

I haven't figured out a way. Just asked StackOverflow - hopefully they'll have an answer.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheTestUtils.java, line 2
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28127#file28127line2>
bq.  >
bq.  >     2011

Fixed.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/io/hfile/CacheStats.java, line 1
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28128#file28128line1>
bq.  >
bq.  >     license

Fixed.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheTestUtils.java, line 41
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28127#file28127line41>
bq.  >
bq.  >     whitespace

Fixed.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/io/hfile/CacheStats.java, line 5
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28128#file28128line5>
bq.  >
bq.  >     class comment

Added


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java, line 39
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28129#file28129line39>
bq.  >
bq.  >     Would it make sense to have DoubleBlockCache be more generic?  Does it need
to be fixed with these two types or could it take two BlockCache's and they are executed in
the order they are given in (just need to be clear in doc).
bq.  >     
bq.  >     If this was generic, it could be reused for various multi-level caches (like
an underlying cache with compressed blocks and one above it with uncompressed blocks)

It can be made more generic easily. I just haven't done it. Will do though - just create a
constructor that takes two BlockCaches?


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java, line 66
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28129#file28129line66>
bq.  >
bq.  >     longer than 80 chars

fixed.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java, lines 77-79
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28129#file28129line77>
bq.  >
bq.  >     This seems like a behavior that we may not always want.
bq.  >     
bq.  >     If we made this class generic, could we have some kind of policy we initiate
it with?  (like default cache in level one, if accessed in level one, cache in level two,
etc?)
bq.  >     
bq.  >     we're going to always be double-storing anything so that the offHeap true capacity
is (totalOffHeap - totalOnHeap).  in some cases, we might want to cache on heap first and
then if evicted we cache off heap, or maybe we want it to work more like the existing LRU
(first read goes into off heap, second read upgrades it to the on heap cache and removes from
the off heap)

I was thinking of sending all initial caches to the off heap cache, giving it an inbuilt scan
resistance, but LRUBlockCache already does partitioning to deal with scans.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java, line 115
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28129#file28129line115>
bq.  >
bq.  >     this is going to make for some weird stats?  seems like we may need to actually
expose the stats of each underlying cache rather than both?  (or both and separate).  it's
going to be difficult to understand what's happening when the hit and eviction stats cover
both.

The idea was the combined evicted stats of both, but yeah, CacheStats are implemented for
both LruBlockCache and SlabCache, so exposing both might be a good idea.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java, line 74
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28132#file28132line74>
bq.  >
bq.  >     huh?

Just a typo. Fixed. Don't know why that line is there.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java, line 84
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28132#file28132line84>
bq.  >
bq.  >     line > 80 chars

fixed.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java, lines 122-125
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28134#file28134line122>
bq.  >
bq.  >     seems like an odd behavior.  this method is kinda non-deterministic from the
caller POV, we have no idea whether this passed or failed

If the size of the item being cached is too big to fit into any slab, we can't cache it. Again,
I'm moving away from MemCached's model, so this should work better.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java, line 151
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28134#file28134line151>
bq.  >
bq.  >     is this really a cache miss?  this cache will never take this block.

Good point. Taking that part out.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java, line 155
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28134#file28134line155>
bq.  >
bq.  >     but here we need to check for null and increment cache miss rather than hit
if it is null

Yup, doing so as well.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java, lines 170-173
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28134#file28134line170>
bq.  >
bq.  >     this non-javadoc comment is not necessary, just the @Override has been normal
format in hbase

Fixed.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java, line 269
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28134#file28134line269>
bq.  >
bq.  >     should this really be a bunch of INFO logs?  Could we have these exported as
metrics instead?

Will take care of this next pass.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabItemEvictionWatcher.java,
line 1
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28135#file28135line1>
bq.  >
bq.  >     license

added.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java, lines 374-375
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28137#file28137line374>
bq.  >
bq.  >     formatting looks a little strange here (missing spaces and such)

fixed.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSingleSlabCache.java, line
28
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28138#file28138line28>
bq.  >
bq.  >     This test looks more specific than testing "the concurrent LruBlockCache"

Woah, typo, fixed.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSingleSlabCache.java, line
34
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28138#file28138line34>
bq.  >
bq.  >     you are missing a space here and throughout this file and some others... our
format is to always have a space before a {

fixed.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSlab.java, line 1
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28139#file28139line1>
bq.  >
bq.  >     license

added.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSingleSlabCache.java, line
2
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28138#file28138line2>
bq.  >
bq.  >     2011

Fixed.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java, line 376
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28137#file28137line376>
bq.  >
bq.  >     line > 80 chars

Fixed.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java, lines 63-65
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28134#file28134line63>
bq.  >
bq.  >     Why these ratios?  At the least, this should all be configurable (even if just
in code and undocumented).
bq.  >     
bq.  >     Do we need to always pre-allocate everything and determine the block/slab sizes
and all that?  The design seems inflexible because it's all determine during construction
rather than being adaptive.
bq.  >     
bq.  >     I'm okay with the first iteration not being awesome and auto-tuning but this
layered cache design seems to make it hard to change anything once it's instantiated.

Its sort of expensive to change cache sizes with DirectByteBuffer after instantiation. I have
a plan to restructure this away from MemCached's model though.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java, line 37
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28129#file28129line37>
bq.  >
bq.  >     whitespace here and throughout this file

Removed.


bq.  On 2011-07-29 17:41:50, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java, line 48
bq.  > <https://reviews.apache.org/r/1214/diff/1/?file=28134#file28134line48>
bq.  >
bq.  >     I'm not totally clear on why the SlabCache contains a bunch of SingleSizeCaches.
 Why do you need to layer BlockCaches on top of BlockCaches?  You'll have one slab per size
rather than one cache per size?  Can you not pass the right evictor callback in so it goes
back to the right slab?

I'm using MemCached's allocation model - although I have one written that does it in a wholly
different manner. But MemCached gets around fragmentation by allocating a certain range of
sizes to a Single Slab.

If something is smaller than the max size of a particular slab, the extra space is just wasted.
Each SingleSizeCache can cache things up to a certain size. Things above that size cannot
be cached, and things below that size will waste space.


- Li


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1214/#review1211
-----------------------------------------------------------


On 2011-07-30 00:39:48, Li Pi wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/1214/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-07-30 00:39:48)
bq.  
bq.  
bq.  Review request for hbase, Todd Lipcon, Ted Yu, Michael Stack, Jonathan Gray, and Li Pi.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Review request - I apparently can't edit tlipcon's earlier posting of my diff, so creating
a new one.
bq.  
bq.  
bq.  This addresses bug HBase-4027.
bq.      https://issues.apache.org/jira/browse/HBase-4027
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    conf/hbase-env.sh 2d55d27 
bq.    src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java 509121d 
bq.    src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheTestUtils.java PRE-CREATION

bq.    src/main/java/org/apache/hadoop/hbase/io/hfile/CacheStats.java PRE-CREATION 
bq.    src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java PRE-CREATION 
bq.    src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java ecab7ca 
bq.    src/main/java/org/apache/hadoop/hbase/io/hfile/SimpleBlockCache.java 150f54f 
bq.    src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java PRE-CREATION

bq.    src/main/java/org/apache/hadoop/hbase/io/hfile/slab/Slab.java PRE-CREATION 
bq.    src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java PRE-CREATION 
bq.    src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabItemEvictionWatcher.java PRE-CREATION

bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1d5e3fa 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java b600020 
bq.    src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSingleSlabCache.java PRE-CREATION

bq.    src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSlab.java PRE-CREATION 
bq.    src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSlabCache.java PRE-CREATION

bq.  
bq.  Diff: https://reviews.apache.org/r/1214/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  Ran benchmarks against it in HBase standalone mode. Wrote test cases for all classes,
multithreaded test cases exist for the cache.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Li
bq.  
bq.



> Enable direct byte buffers LruBlockCache
> ----------------------------------------
>
>                 Key: HBASE-4027
>                 URL: https://issues.apache.org/jira/browse/HBASE-4027
>             Project: HBase
>          Issue Type: Improvement
>            Reporter: Jason Rutherglen
>            Assignee: Li Pi
>            Priority: Minor
>         Attachments: 4027-v5.diff, 4027v7.diff, HBase-4027 (1).pdf, HBase-4027.pdf, hbase-4027v6.diff,
slabcachepatch.diff, slabcachepatchv2.diff, slabcachepatchv3.1.diff, slabcachepatchv3.2.diff,
slabcachepatchv3.diff, slabcachepatchv4.5.diff, slabcachepatchv4.diff
>
>
> Java offers the creation of direct byte buffers which are allocated outside of the heap.
> They need to be manually free'd, which can be accomplished using an documented {{clean}}
method.
> The feature will be optional.  After implementing, we can benchmark for differences in
speed and garbage collection observances.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message