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

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



bq.  On 2011-08-09 22:05:13, Todd Lipcon wrote:
bq.  > 
src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java, lines 
118-119
bq.  > <https://reviews.apache.org/r/1214/diff/8/?file=31771#file31771line118>
bq.  >
bq.  >     I think there's a bug here if you have multiple users hammering the 
same contentBlock -- two people can get to "rewind()" at the same time. You 
probably need synchronized(contentBlock) around these two lines. See if you can 
add a unit test which puts just one block in the cache and starts several 
threads which hammer it - I bet you eventually one of the blocks comes back 
returned as all 0x0000
bq.  
bq.  Li Pi wrote:
bq.      changed it to     returnBlock.put(contentBlock.duplicate()) instead.
bq.  
bq.  Todd Lipcon wrote:
bq.      don't you need to duplicate it before you call rewind? also, did you 
add a test that catches this bug, in case there are other similar bugs that I 
didn't spot?

Since we duplicate it, we don't need to rewind after using it. The initial copy 
is already rewound, so we just duplicate it, do our put, and then discard it. 
TestHammerSingleKey has been written.


bq.  On 2011-08-09 22:05:13, Todd Lipcon wrote:
bq.  > 
src/test/java/org/apache/hadoop/hbase/io/hfile/SingleSizeCacheTestUtils.java, 
line 45
bq.  > <https://reviews.apache.org/r/1214/diff/8/?file=31780#file31780line45>
bq.  >
bq.  >     hrm, this is identical to the other method?
bq.  
bq.  Li Pi wrote:
bq.      Ones for SingleSizeCache, which takes ByteBuffers, the other is for a 
BlockCache, which (hypothetically) takes anything with HeapSize.
bq.  
bq.  Todd Lipcon wrote:
bq.      but you should be able to extract a method, even if the method has to 
take a type parameter. too much dup code

We have a Cacheable interface now. No more dup code.


- Li


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


On 2011-08-18 08:31:59, 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-08-18 08:31:59)
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 2d4002c 
bq.    src/main/java/org/apache/hadoop/hbase/io/hfile/CacheStats.java 
PRE-CREATION 
bq.    src/main/java/org/apache/hadoop/hbase/io/hfile/Cacheable.java 
PRE-CREATION 
bq.    src/main/java/org/apache/hadoop/hbase/io/hfile/CachedBlock.java 3b130d8 
bq.    src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java 
PRE-CREATION 
bq.    src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java 097dc50 
bq.    src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java 
1338453 
bq.    src/main/java/org/apache/hadoop/hbase/io/hfile/SimpleBlockCache.java 
886c31d 
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 
e2c6c93 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 
7b7bf73 
bq.    src/main/java/org/apache/hadoop/hbase/util/DirectMemoryUtils.java 
PRE-CREATION 
bq.    src/test/java/org/apache/hadoop/hbase/io/hfile/CacheTestUtils.java 
PRE-CREATION 
bq.    src/test/java/org/apache/hadoop/hbase/io/hfile/TestCachedBlockQueue.java 
1ad2ece 
bq.    src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruBlockCache.java 
f0a9832 
bq.    
src/test/java/org/apache/hadoop/hbase/io/hfile/slab/TestSingleSizeCache.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.    src/test/java/org/apache/hadoop/hbase/regionserver/TestMemStoreLAB.java 
d7e43a0 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 
4387170 
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, HBase4027v8.diff, HBase4027v9.diff, hbase-4027-v10.5.diff, 
> hbase-4027-v10.diff, hbase-4027v10.6.diff, hbase-4027v6.diff, 
> hbase4027v11.5.diff, hbase4027v11.6.diff, hbase4027v11.7.diff, 
> hbase4027v11.diff, hbase4027v12.1.diff, hbase4027v12.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

        

Reply via email to