[
https://issues.apache.org/jira/browse/HBASE-4027?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13089148#comment-13089148
]
[email protected] commented on HBASE-4027:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1214/#review1594
-----------------------------------------------------------
Looking good! Still some lines > 80 chars and whitespace to fix, but otherwise
not much to comment on. As we discussed, I find the Cacheable interface name a
bit confusing. Maybe rename it, or just fill the classes and methods with
awesome javadoc that explains how it all works :)
Also, should probably add a section to the book about this? Some of these
parameters are pretty important and the defaults could be wasteful for some of
the default use cases (for example, always giving 20% of heap to blocks > 1.1X
block size)
src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java
<https://reviews.apache.org/r/1214/#comment3591>
is it required still that the block contents be wrapped in a ByteBuffer?
does Cacheable enforce this? if not, should this javadoc be updated?
src/main/java/org/apache/hadoop/hbase/io/hfile/CacheStats.java
<https://reviews.apache.org/r/1214/#comment3592>
extra newline (maybe move it to below the public class).
also missing a space after CacheStats
src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java
<https://reviews.apache.org/r/1214/#comment3594>
looks like this javadoc is accidentally applied on the CacheStats private
var instead of constructor
src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java
<https://reviews.apache.org/r/1214/#comment3595>
should this be moved somewhere else? seems strange to have all this code
up in the top of the class in the middle of class variable members
src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java
<https://reviews.apache.org/r/1214/#comment3596>
is this necessary since we throw a runtime exception? could this leave the
RS in a weird state? should we actually halt or abort? or will that happen
when this gets thrown?
src/main/java/org/apache/hadoop/hbase/io/hfile/slab/Slab.java
<https://reviews.apache.org/r/1214/#comment3599>
i think this is the only place you use the term "chunks". should we just
call the slab chunks blocks? or should we call them chunks everywhere to avoid
confusion with hfile blocks? although they are kinda the same :)
src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java
<https://reviews.apache.org/r/1214/#comment3597>
javadoc
src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabItemEvictionWatcher.java
<https://reviews.apache.org/r/1214/#comment3600>
add @param @return javadoc
- Jonathan
On 2011-08-22 23:09:44, 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-22 23:09:44)
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. CHANGES.txt 0bb7c65
bq. conf/hbase-env.sh 2d55d27
bq. src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java bfd863e
bq.
src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheColumnFamilySummary.java
34513f1
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/CacheableDeserializer.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
ae75299
bq. src/main/java/org/apache/hadoop/hbase/io/hfile/SimpleBlockCache.java
7ce4f14
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/ipc/HRegionInterface.java 8f5600d
bq. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
a817c37
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/main/java/org/apache/hadoop/hbase/util/FSUtils.java d6fa01b
bq. src/test/java/org/apache/hadoop/hbase/io/hfile/CacheTestUtils.java
PRE-CREATION
bq.
src/test/java/org/apache/hadoop/hbase/io/hfile/TestBlockCacheColumnFamilySummary.java
cc4abc6
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.
src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java
f8a854c
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-4027v13.1.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
