[
https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13120556#comment-13120556
]
[email protected] commented on HBASE-4422:
------------------------------------------------------
bq. On 2011-10-04 21:52:34, Michael Stack wrote:
bq. >
/src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java, line
278
bq. > <https://reviews.apache.org/r/2089/diff/5/?file=47786#file47786line278>
bq. >
bq. > Don't we still need this?
No, this was a hack to inject the block cache into the writer. It gets the
CacheConfig now.
bq. On 2011-10-04 21:52:34, Michael Stack wrote:
bq. > /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java, line 74
bq. > <https://reviews.apache.org/r/2089/diff/5/?file=47787#file47787line74>
bq. >
bq. > This is good defining these keys in here instead of up in HConstants
or somewhere
Yeah, they were previously scattered across 4 files.
bq. On 2011-10-04 21:52:34, Michael Stack wrote:
bq. > /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java, line 32
bq. > <https://reviews.apache.org/r/2089/diff/5/?file=47787#file47787line32>
bq. >
bq. > Its cache and a cache config? Seems a bit odd that you come here to
get the block cache instance too. I'd think that'd be a CacheFactory that took
a CacheConfig.
The cache used to live in StoreFile. I saw this as an improvement. It's still
this horrid static thing, just in a better place than before. Eventually we
should probably actually instantiate the block cache up in HRS. As it is
today, we have one block cache per JVM so even our multi RS unit tests are
sharing a block cache.
bq. On 2011-10-04 21:52:34, Michael Stack wrote:
bq. > /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java, line
109
bq. > <https://reviews.apache.org/r/2089/diff/5/?file=47787#file47787line109>
bq. >
bq. > Future feature?
Coming very shortly after this is committed!
bq. On 2011-10-04 21:52:34, Michael Stack wrote:
bq. > /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java, line 341
bq. > <https://reviews.apache.org/r/2089/diff/5/?file=47798#file47798line341>
bq. >
bq. > Is this right? Its a static or something? Can't pass Configuration
and/or family? Just defaults?
Addressing this from Mikhail's review.
bq. On 2011-10-04 21:52:34, Michael Stack wrote:
bq. > /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java, line
358
bq. > <https://reviews.apache.org/r/2089/diff/5/?file=47790#file47790line358>
bq. >
bq. > ditto
You either cacheConf.getBlockCache() or you CacheConf.getBlockCache(conf)
(previously we were passing it all the way in, now we just pass in CacheConfig)
This seems to make much more sense and if we actually do eventually have
multiple block caches (or at least non-static so multiple in a JVM) we'll want
to just be able to hold it in the CacheConfig rather than need to carry in a
CacheConfig and a separate BlockCache.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2089/#review2320
-----------------------------------------------------------
On 2011-10-04 02:50:58, Jonathan Gray wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2089/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-10-04 02:50:58)
bq.
bq.
bq. Review request for hbase, Dhruba Borthakur, Michael Stack, and Li Pi.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Creates a new CacheConfig class and moves almost everything block cache
related into this single class. Adding new configuration params and booleans
and such should be much better.
bq.
bq. All tests are NOT passing yet, still working on it, but wanted to have
something up today. Basically "code complete" but broken :)
bq.
bq.
bq. This addresses bug HBASE-4422.
bq. https://issues.apache.org/jira/browse/HBASE-4422
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. /src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java
1178676
bq. /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java
1178676
bq. /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java
1178676
bq. /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
PRE-CREATION
bq. /src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java 1178676
bq. /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java
1178676
bq. /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java
1178676
bq. /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
1178676
bq. /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java
1178676
bq. /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
1178676
bq. /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java
1178676
bq.
/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
1178676
bq. /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1178676
bq. /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
1178676
bq. /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1178676
bq. /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
1178676
bq. /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java
1178676
bq. /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1178676
bq. /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java
1178676
bq. /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java
1178676
bq. /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1178676
bq. /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java
1178676
bq. /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1178676
bq. /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java
1178676
bq.
/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java
1178676
bq. /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java
1178676
bq. /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java
1178676
bq. /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java
1178676
bq. /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java
1178676
bq. /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1178676
bq.
/src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java
1178676
bq.
/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
1178676
bq. /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java
1178676
bq.
/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java
1178676
bq.
/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java
1178676
bq.
/src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java
1178676
bq. /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
1178676
bq.
/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java
1178676
bq.
/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java
1178676
bq.
bq. Diff: https://reviews.apache.org/r/2089/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Still working through some tests that aren't passing.
bq.
bq.
bq. Thanks,
bq.
bq. Jonathan
bq.
bq.
> Move block cache parameters and references into single CacheConf class
> ----------------------------------------------------------------------
>
> Key: HBASE-4422
> URL: https://issues.apache.org/jira/browse/HBASE-4422
> Project: HBase
> Issue Type: Improvement
> Components: io
> Reporter: Jonathan Gray
> Assignee: Jonathan Gray
> Fix For: 0.92.0
>
>
> From StoreFile down to HFile, we currently use a boolean argument for each of
> the various block cache configuration parameters that exist. The number of
> parameters is going to continue to increase as we look at compressed cache,
> delta encoding, and more specific L1/L2 configuration. Every new config
> currently requires changing many constructors because it introduces a new
> boolean.
> We should move everything into a single class so that modifications are much
> less disruptive.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
