You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "Jonathan Gray (JIRA)" <ji...@apache.org> on 2011/09/16 22:32:08 UTC

[jira] [Created] (HBASE-4422) Move block cache parameters and references into single CacheConf class

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.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Phabricator (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13209066#comment-13209066 ] 

Phabricator commented on HBASE-4422:
------------------------------------

mbautin has abandoned the revision "[jira] [HBASE-4422] [89-fb] Move block cache parameters and references into single CacheConfig class".

REVISION DETAIL
  https://reviews.facebook.net/D1341

                
> 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
>
>         Attachments: CacheConfig92-v8.patch, D1341.1.patch, HBASE-4422-FINAL-branch92.patch, HBASE-4422-FINAL-trunk.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13118323#comment-13118323 ] 

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


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

(Updated 2011-09-30 19:36:04.480088)


Review request for hbase, Dhruba Borthakur, Michael Stack, and Li Pi.


Changes
-------

More bug fixes and removed some debug output.

Unit tests are passing for me when I run them in eclipse.  For some reason, lots of stuff is frozen when I run it on my remote dev box but having problems with and without this patch.

Appreciate reviews and unit test runs!  Thanks


Summary
-------

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.

All tests are NOT passing yet, still working on it, but wanted to have something up today.  Basically "code complete" but broken :)


This addresses bug HBASE-4422.
    https://issues.apache.org/jira/browse/HBASE-4422


Diffs (updated)
-----

  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1177760 
  /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1177760 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1177760 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1177760 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1177760 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1177760 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1177760 
  /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1177760 
  /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1177760 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1177760 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1177760 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1177760 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1177760 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1177760 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1177760 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1177760 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1177760 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1177760 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1177760 
  /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1177760 
  /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1177760 
  /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1177760 
  /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1177760 
  /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1177760 
  /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1177760 
  /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1177760 
  /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1177760 
  /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1177760 
  /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1177760 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1177760 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1177760 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1177760 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1177760 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1177760 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java 1177760 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1177760 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java PRE-CREATION 
  /src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java 1177760 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1177760 

Diff: https://reviews.apache.org/r/2089/diff


Testing
-------

Still working through some tests that aren't passing.


Thanks,

Jonathan


                
> 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

        

[jira] [Resolved] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Jonathan Gray (Resolved) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jonathan Gray resolved HBASE-4422.
----------------------------------

      Resolution: Fixed
    Release Note: Relocates all cache configuration options and constants into a new CacheConfig class.  Modifies lots of constructors from Store/StoreFile down to HFile.
    Hadoop Flags: Reviewed

Committed to trunk and branch.  Thanks everyone for all the reviews.  Thanks Mikhail for all the back and forth!
                
> 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
>
>         Attachments: CacheConfig92-v8.patch, HBASE-4422-FINAL-branch92.patch, HBASE-4422-FINAL-trunk.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13116935#comment-13116935 ] 

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


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


Cool... This makes it much simpler in the future.
Are you still planning to fold HBASE-4496 into this?


/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java
<https://reviews.apache.org/r/2089/#comment4981>

    Hmm... The new code does not honor the cacheBlock flag that was passed into the method.



/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java
<https://reviews.apache.org/r/2089/#comment4982>

    Same here. blockCache flag is not honored.


- Lars


On 2011-09-28 19:56:14, 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-09-28 19:56:14)
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 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1177030 
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 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1177030 
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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13119580#comment-13119580 ] 

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


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


Looks good from here... I'll do some testing with the patch applied.


/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
<https://reviews.apache.org/r/2089/#comment5247>

    That'd be pretty awesome :)


- Lars


On 2011-10-03 18:19:34, 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-03 18:19:34)
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/util/BloomFilterFactory.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1178474 
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 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1178474 
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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13120505#comment-13120505 ] 

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


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


Looks good.  Nice cleanup.  Just one issue w/ modelling where we ask CacheConfig for Cache instance -- seems 'off'.


/src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java
<https://reviews.apache.org/r/2089/#comment5342>

    CacheConfig has a Configuration?



/src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java
<https://reviews.apache.org/r/2089/#comment5346>

    Don't we still need this?



/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
<https://reviews.apache.org/r/2089/#comment5347>

    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.



/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
<https://reviews.apache.org/r/2089/#comment5349>

    This is good defining these keys in here instead of up in HConstants or somewhere



/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
<https://reviews.apache.org/r/2089/#comment5350>

    Future feature?



/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java
<https://reviews.apache.org/r/2089/#comment5351>

    Yeah, seems odd asking CacheConfig for the Cache.



/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java
<https://reviews.apache.org/r/2089/#comment5352>

    ditto



/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
<https://reviews.apache.org/r/2089/#comment5354>

    Is this right?  Its a static or something?  Can't pass Configuration and/or family?  Just defaults?


- Michael


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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Phabricator (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13196360#comment-13196360 ] 

Phabricator commented on HBASE-4422:
------------------------------------

mbautin has commented on the revision "[jira] [HBASE-4422] [89-fb] Move block cache parameters and references into single CacheConfig class".

  @Kannan: thanks for reviewing. See my reply inline.

INLINE COMMENTS
  src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java:294 Yes, I agree there is a discrepancy here. This also exists in trunk. However, the only difference this makes is in the LruBlockCache's CacheStats framework which we do not use form metrics. I added this as an action item to address in https://issues.apache.org/jira/browse/HBASE-5012.

REVISION DETAIL
  https://reviews.facebook.net/D1341

                
> 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
>
>         Attachments: CacheConfig92-v8.patch, D1341.1.patch, HBASE-4422-FINAL-branch92.patch, HBASE-4422-FINAL-trunk.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13116924#comment-13116924 ] 

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


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


RB couldn't handle the diffs to StoreFile, something weird there.

I'll look at this again when the tests are working.


/src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java
<https://reviews.apache.org/r/2089/#comment4979>

    CacheConfig could extend Configuration? It's only extra constants and constructors, really.



/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java
<https://reviews.apache.org/r/2089/#comment4980>

    See above


- Andrew


On 2011-09-28 19:56:14, 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-09-28 19:56:14)
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 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1177030 
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 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1177030 
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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Ted Yu (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13120644#comment-13120644 ] 

Ted Yu commented on HBASE-4422:
-------------------------------

+1 on integrating to 0.92 branch.
                
> 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
>
>         Attachments: CacheConfig92-v8.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Phabricator (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13196505#comment-13196505 ] 

Phabricator commented on HBASE-4422:
------------------------------------

mbautin has commented on the revision "[jira] [HBASE-4422] [89-fb] Move block cache parameters and references into single CacheConfig class".

INLINE COMMENTS
  src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java:294 s/form metrics/for metrics/

REVISION DETAIL
  https://reviews.facebook.net/D1341

                
> 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
>
>         Attachments: CacheConfig92-v8.patch, D1341.1.patch, HBASE-4422-FINAL-branch92.patch, HBASE-4422-FINAL-trunk.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Jonathan Gray (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13120692#comment-13120692 ] 

Jonathan Gray commented on HBASE-4422:
--------------------------------------

I have looked at 3446 more.  I'm happy with it and confident it makes things better.  Will give the +1.

Re: getting the cache instance from CacheConf, i'm open to other designs, but this seems best in that we only need one argument for all the caching stuff vs. a separate reference for the cache itself.  What did you have in mind?

Maybe CacheConfig + BlockCache should be somehow combined?  Dunno.
                
> 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
>
>         Attachments: CacheConfig92-v8.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "stack (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13120678#comment-13120678 ] 

stack commented on HBASE-4422:
------------------------------

As RM, I will not let this patch into 0.92.  Not unless you review hbase-3446!  Then I might consider it.

+1 on commit to 0.92.  Looks low risk cleanup to me.  I don't like getting the cache instance from the CacheConf so we should file a new issue for that too before it can go in.

Good stuff.
                
> 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
>
>         Attachments: CacheConfig92-v8.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13117546#comment-13117546 ] 

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



bq.  On 2011-09-29 00:50:58, Andrew Purtell wrote:
bq.  > /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java, line 112
bq.  > <https://reviews.apache.org/r/2089/diff/1/?file=46305#file46305line112>
bq.  >
bq.  >     CacheConfig could extend Configuration? It's only extra constants and constructors, really.
bq.  
bq.  Jonathan Gray wrote:
bq.      I considered doing that.  But they seem orthogonal in most of their usage and I think it'd be weird to pass our Configuration all the way down into HFile reading and such.  Changing config values dynamically (like disabling caching for a specific read) would then require changing the base Configuration or would the local booleans in the CacheConfig not match what's in the underlying Configuration?

Fair enough, but it's a common pattern to mutate a Configuration object and then pass it along.


- Andrew


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


On 2011-09-28 19:56:14, 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-09-28 19:56:14)
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 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1177030 
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 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1177030 
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

        

[jira] [Updated] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Jonathan Gray (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jonathan Gray updated HBASE-4422:
---------------------------------

    Attachment: HBASE-4422-FINAL-branch92.patch

Final patch for commit on 92 branch
                
> 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
>
>         Attachments: CacheConfig92-v8.patch, HBASE-4422-FINAL-branch92.patch, HBASE-4422-FINAL-trunk.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Lars Hofhansl (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13120655#comment-13120655 ] 

Lars Hofhansl commented on HBASE-4422:
--------------------------------------

+1 as well, seems low risk.
                
> 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
>
>         Attachments: CacheConfig92-v8.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13119465#comment-13119465 ] 

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


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

(Updated 2011-10-03 18:19:34.483689)


Review request for hbase, Dhruba Borthakur, Michael Stack, and Li Pi.


Changes
-------

Rebased following the commit of HBASE-4496.  Appreciate if people can download the patch and run the test suite and report back what new stuff is failing for you, getting some inconsistent results.


Summary
-------

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.

All tests are NOT passing yet, still working on it, but wanted to have something up today.  Basically "code complete" but broken :)


This addresses bug HBASE-4422.
    https://issues.apache.org/jira/browse/HBASE-4422


Diffs (updated)
-----

  /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1178474 
  /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1178474 
  /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1178474 
  /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1178474 
  /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1178474 
  /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1178474 
  /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1178474 
  /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1178474 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1178474 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1178474 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java PRE-CREATION 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java 1178474 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1178474 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1178474 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1178474 
  /src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java 1178474 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1178474 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1178474 
  /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1178474 

Diff: https://reviews.apache.org/r/2089/diff


Testing
-------

Still working through some tests that aren't passing.


Thanks,

Jonathan


                
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13120544#comment-13120544 ] 

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



bq.  On 2011-10-04 21:41:43, Mikhail Bautin wrote:
bq.  > Sending the second part of comments. Still have not reviewed StoreFile changes. I guess my biggest concern is the initialization of a default CacheConfig in a lot of places (hopefully mostly tests) while it could be initialized based on the available Configuration.

Yeah, it was a concern i had.  I can go through a remove all the empty constructors and require a Configuration and see if I can still get all the tests to pass.


bq.  On 2011-10-04 21:41:43, Mikhail Bautin wrote:
bq.  > /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java, line 113
bq.  > <https://reviews.apache.org/r/2089/diff/5/?file=47801#file47801line113>
bq.  >
bq.  >     Should not there be a way to create a default CacheConf() from a Configuration()? Currently it seems that you are creating a default CacheConfig to pass to getWriteFactory even though you have a conf available, potentially with some cache-related settings. Maybe there should be an overload of HFile.getWriterFactory that takes a conf and creates a default CacheConfig out of it.

I will go through and see if i can clean that up and not use an empty constructor of CacheConfig anywhere


bq.  On 2011-10-04 21:41:43, Mikhail Bautin wrote:
bq.  > /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java, line 72
bq.  > <https://reviews.apache.org/r/2089/diff/5/?file=47804#file47804line72>
bq.  >
bq.  >     nit: trailing whitespace (we probably should remove all of it in one patch someday :)

nipped


bq.  On 2011-10-04 21:41:43, Mikhail Bautin wrote:
bq.  > /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java, line 185
bq.  > <https://reviews.apache.org/r/2089/diff/5/?file=47816#file47816line185>
bq.  >
bq.  >     Should not we move the cast to initialization and change blockCache's type to LruBlockCache?
bq.  >     
bq.  >     Is this method only intended for testing, by the way? If so, should this be indicated in the method name?

There was work done so we didn't need to have LruBlockCache and could use the BlockCache interface.  Unfortunately the BlockCache interface doesn't expose a method to get the count of elements.  I could just add it to the BlockCache interface?


bq.  On 2011-10-04 21:41:43, Mikhail Bautin wrote:
bq.  > /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java, line 118
bq.  > <https://reviews.apache.org/r/2089/diff/5/?file=47805#file47805line118>
bq.  >
bq.  >     Do you want to keep this debug code around?

in a test, doesn't hurt i think (makes sense in context of this being a parameterized test)


- Jonathan


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


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

        

[jira] [Updated] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Jonathan Gray (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jonathan Gray updated HBASE-4422:
---------------------------------

    Attachment: HBASE-4422-FINAL-trunk.patch

Final patch for commit to trunk
                
> 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
>
>         Attachments: CacheConfig92-v8.patch, HBASE-4422-FINAL-branch92.patch, HBASE-4422-FINAL-trunk.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13125366#comment-13125366 ] 

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


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

(Updated 2011-10-11 20:35:51.722038)


Review request for hbase, Dhruba Borthakur, Michael Stack, and Li Pi.


Changes
-------

Updated version rebased on latest 92 branch.  Two new test failures after rebasing...  TestStoreFile.testBloomTypes and TestHFile.testTFileFeatures.

It seems like I'm somehow corrupting data or dropping a flag somewhere?  Not really sure.


Summary
-------

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.

All tests are NOT passing yet, still working on it, but wanted to have something up today.  Basically "code complete" but broken :)


This addresses bug HBASE-4422.
    https://issues.apache.org/jira/browse/HBASE-4422


Diffs (updated)
-----

  /src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java PRE-CREATION 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/SimpleBlockCache.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/util/TestMergeTool.java 1182021 

Diff: https://reviews.apache.org/r/2089/diff


Testing
-------

Still working through some tests that aren't passing.


Thanks,

Jonathan


                
> 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
>
>         Attachments: CacheConfig92-v8.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13124480#comment-13124480 ] 

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


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



/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
<https://reviews.apache.org/r/2089/#comment5653>

    nit: add a space after getBlockCache()



/src/test/java/org/apache/hadoop/hbase/util/TestMergeTool.java
<https://reviews.apache.org/r/2089/#comment5654>

    How is this related to block cache?


- Mikhail


On 2011-10-10 21:08:15, 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-10 21:08:15)
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 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java PRE-CREATION 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/SimpleBlockCache.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/util/TestMergeTool.java 1179008 
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
>
>         Attachments: CacheConfig92-v8.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13116720#comment-13116720 ] 

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


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

Review request for hbase, Dhruba Borthakur, Michael Stack, and Li Pi.


Summary
-------

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.

All tests are NOT passing yet, still working on it, but wanted to have something up today.  Basically "code complete" but broken :)


This addresses bug HBASE-4422.
    https://issues.apache.org/jira/browse/HBASE-4422


Diffs
-----


Diff: https://reviews.apache.org/r/2089/diff


Testing
-------

Still working through some tests that aren't passing.


Thanks,

Jonathan


                
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13119882#comment-13119882 ] 

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


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

(Updated 2011-10-04 02:50:58.274414)


Review request for hbase, Dhruba Borthakur, Michael Stack, and Li Pi.


Changes
-------

Minor fixes to some tests.  I have all tests passing now!


Summary
-------

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.

All tests are NOT passing yet, still working on it, but wanted to have something up today.  Basically "code complete" but broken :)


This addresses bug HBASE-4422.
    https://issues.apache.org/jira/browse/HBASE-4422


Diffs (updated)
-----

  /src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java 1178676 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1178676 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1178676 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java PRE-CREATION 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java 1178676 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1178676 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1178676 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1178676 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1178676 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1178676 
  /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1178676 
  /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1178676 
  /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1178676 
  /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1178676 
  /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1178676 
  /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1178676 
  /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1178676 
  /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1178676 
  /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1178676 

Diff: https://reviews.apache.org/r/2089/diff


Testing
-------

Still working through some tests that aren't passing.


Thanks,

Jonathan


                
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13125543#comment-13125543 ] 

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


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

Ship it!


1032 unit tests passed, 0 failed, 16 skipped (I still don't know why Maven skips tests and whether it is a problem -- any idea?)
I think this patch is good to go.

- Mikhail


On 2011-10-11 22:18:32, 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-11 22:18:32)
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 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java PRE-CREATION 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/SimpleBlockCache.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1182021 
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
>
>         Attachments: CacheConfig92-v8.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Phabricator (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13209165#comment-13209165 ] 

Phabricator commented on HBASE-4422:
------------------------------------

mbautin has committed the revision "[jira] [HBASE-4422] [89-fb] Move block cache parameters and references into single CacheConfig class".

REVISION DETAIL
  https://reviews.facebook.net/D1341

COMMIT
  https://reviews.facebook.net/rHBASEEIGHTNINEFBBRANCH1239901

                
> 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
>
>         Attachments: CacheConfig92-v8.patch, D1341.1.patch, HBASE-4422-FINAL-branch92.patch, HBASE-4422-FINAL-trunk.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13124481#comment-13124481 ] 

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



bq.  On 2011-10-10 21:19:18, Mikhail Bautin wrote:
bq.  >

Also, the /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java diff is still not showing up.


- Mikhail


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


On 2011-10-10 21:08:15, 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-10 21:08:15)
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 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java PRE-CREATION 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/SimpleBlockCache.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/util/TestMergeTool.java 1179008 
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
>
>         Attachments: CacheConfig92-v8.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13120490#comment-13120490 ] 

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


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



/src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java
<https://reviews.apache.org/r/2089/#comment5323>

    is .toString() necessary?



/src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java
<https://reviews.apache.org/r/2089/#comment5324>

    This simplifies things a lot.



/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
<https://reviews.apache.org/r/2089/#comment5325>

    Why not make this private?



/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
<https://reviews.apache.org/r/2089/#comment5326>

    Should this be suffixed with _KEY for consistency with other conf key constants in this class?



/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
<https://reviews.apache.org/r/2089/#comment5327>

    Is this constructor ever used outside of CacheConfig itself? It looks like a potential source of errors, because the caller has to get the order of 7 boolean arguments right.



/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
<https://reviews.apache.org/r/2089/#comment5328>

    Why is this not private?



/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
<https://reviews.apache.org/r/2089/#comment5329>

    nit: space between "if" and the parenthesis :)



/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java
<https://reviews.apache.org/r/2089/#comment5330>

    This is really simplifying a lot of code :)



/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java
<https://reviews.apache.org/r/2089/#comment5331>

    It might be worth it to rename isInMemory() to useInMemoryCacheBucket() or isInMemoryCF(). The current name is too general.



/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java
<https://reviews.apache.org/r/2089/#comment5332>

    Will this always return false if there is no block cache?



/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
<https://reviews.apache.org/r/2089/#comment5334>

    cacheBlock should probably be set to false if !cacheBlock.isBlockCacheEnabled()



/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
<https://reviews.apache.org/r/2089/#comment5333>

    cacheBlock is not set to false if !cacheConf.isBlockCacheEnabled(). Could this result in an NPE?



/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
<https://reviews.apache.org/r/2089/#comment5335>

    It would be good for readability and maybe for performance to assign cacheConf.shouldCacheIndexesOnWrite() to a local variable.


- Mikhail


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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Phabricator (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13208954#comment-13208954 ] 

Phabricator commented on HBASE-4422:
------------------------------------

mbautin has commented on the revision "[jira] [HBASE-4422] [89-fb] Move block cache parameters and references into single CacheConfig class".

  Committed, closing the diff. Phabricator is not detecting committed diffs for some reason.

REVISION DETAIL
  https://reviews.facebook.net/D1341

                
> 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
>
>         Attachments: CacheConfig92-v8.patch, D1341.1.patch, HBASE-4422-FINAL-branch92.patch, HBASE-4422-FINAL-trunk.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13116742#comment-13116742 ] 

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


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

(Updated 2011-09-28 19:56:14.698336)


Review request for hbase, Dhruba Borthakur, Michael Stack, and Li Pi.


Changes
-------

Diff attached now.


Summary
-------

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.

All tests are NOT passing yet, still working on it, but wanted to have something up today.  Basically "code complete" but broken :)


This addresses bug HBASE-4422.
    https://issues.apache.org/jira/browse/HBASE-4422


Diffs (updated)
-----

  /src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java 1177030 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1177030 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1177030 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java PRE-CREATION 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java 1177030 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java 1177030 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1177030 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1177030 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1177030 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1177030 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1177030 
  /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1177030 
  /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1177030 
  /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1177030 
  /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1177030 
  /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1177030 
  /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1177030 
  /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1177030 
  /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1177030 
  /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1177030 

Diff: https://reviews.apache.org/r/2089/diff


Testing
-------

Still working through some tests that aren't passing.


Thanks,

Jonathan


                
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "stack (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13106797#comment-13106797 ] 

stack commented on HBASE-4422:
------------------------------

+1

> 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.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13124455#comment-13124455 ] 

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


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

(Updated 2011-10-10 20:19:30.766633)


Review request for hbase, Dhruba Borthakur, Michael Stack, and Li Pi.


Changes
-------

After another review from Mikhail, this version brings back the local block cache instance variable and makes all of the static variables and methods private.


Summary
-------

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.

All tests are NOT passing yet, still working on it, but wanted to have something up today.  Basically "code complete" but broken :)


This addresses bug HBASE-4422.
    https://issues.apache.org/jira/browse/HBASE-4422


Diffs (updated)
-----

  /src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java PRE-CREATION 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/SimpleBlockCache.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/util/TestMergeTool.java 1179008 

Diff: https://reviews.apache.org/r/2089/diff


Testing
-------

Still working through some tests that aren't passing.


Thanks,

Jonathan


                
> 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
>
>         Attachments: CacheConfig92-v8.patch
>
>
> 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

        

[jira] [Updated] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Jonathan Gray (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jonathan Gray updated HBASE-4422:
---------------------------------

    Attachment: CacheConfig92-v8.patch

Most recent patch from RB.  All tests are passing.

I will definitely be pulling this into our internal 92 branch and will be building some other stuff on top of it (compressed cache, etc).  It will probably be easiest to integrate the DeltaEncoding stuff with this in place as well.

For that reason, I'd be interested in getting it into 92 branch, it should be low risk.
                
> 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
>
>         Attachments: CacheConfig92-v8.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Li Pi (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13106885#comment-13106885 ] 

Li Pi commented on HBASE-4422:
------------------------------

+1 as well.



> 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.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13117456#comment-13117456 ] 

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



bq.  On 2011-09-29 00:50:58, Andrew Purtell wrote:
bq.  > /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java, line 112
bq.  > <https://reviews.apache.org/r/2089/diff/1/?file=46305#file46305line112>
bq.  >
bq.  >     CacheConfig could extend Configuration? It's only extra constants and constructors, really.

I considered doing that.  But they seem orthogonal in most of their usage and I think it'd be weird to pass our Configuration all the way down into HFile reading and such.  Changing config values dynamically (like disabling caching for a specific read) would then require changing the base Configuration or would the local booleans in the CacheConfig not match what's in the underlying Configuration?


- Jonathan


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


On 2011-09-28 19:56:14, 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-09-28 19:56:14)
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 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1177030 
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 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1177030 
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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13120550#comment-13120550 ] 

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



bq.  On 2011-10-04 21:41:43, Mikhail Bautin wrote:
bq.  > /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java, line 185
bq.  > <https://reviews.apache.org/r/2089/diff/5/?file=47816#file47816line185>
bq.  >
bq.  >     Should not we move the cast to initialization and change blockCache's type to LruBlockCache?
bq.  >     
bq.  >     Is this method only intended for testing, by the way? If so, should this be indicated in the method name?
bq.  
bq.  Jonathan Gray wrote:
bq.      There was work done so we didn't need to have LruBlockCache and could use the BlockCache interface.  Unfortunately the BlockCache interface doesn't expose a method to get the count of elements.  I could just add it to the BlockCache interface?

Okay I added getBlockCount() to BlockCache interface.


- Jonathan


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


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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Phabricator (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13196361#comment-13196361 ] 

Phabricator commented on HBASE-4422:
------------------------------------

Kannan has accepted the revision "[jira] [HBASE-4422] [89-fb] Move block cache parameters and references into single CacheConfig class".

  Sounds good. Thanks for clarifying.

  LGTM!

REVISION DETAIL
  https://reviews.facebook.net/D1341

                
> 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
>
>         Attachments: CacheConfig92-v8.patch, D1341.1.patch, HBASE-4422-FINAL-branch92.patch, HBASE-4422-FINAL-trunk.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Phabricator (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13194497#comment-13194497 ] 

Phabricator commented on HBASE-4422:
------------------------------------

Kannan has commented on the revision "[jira] [HBASE-4422] [89-fb] Move block cache parameters and references into single CacheConfig class".

  looks great. Only one question/comment inline..

INLINE COMMENTS
  src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java:294 here, we have gone from using "cacheBlock" to cacheConf.shouldCacheDataOnRead() based check, but in line 330 below, we are using both. Why is that?

REVISION DETAIL
  https://reviews.facebook.net/D1341

                
> 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
>
>         Attachments: CacheConfig92-v8.patch, D1341.1.patch, HBASE-4422-FINAL-branch92.patch, HBASE-4422-FINAL-trunk.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Jonathan Gray (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13120693#comment-13120693 ] 

Jonathan Gray commented on HBASE-4422:
--------------------------------------

I have looked at 3446 more.  I'm happy with it and confident it makes things better.  Will give the +1.

Re: getting the cache instance from CacheConf, i'm open to other designs, but this seems best in that we only need one argument for all the caching stuff vs. a separate reference for the cache itself.  What did you have in mind?

Maybe CacheConfig + BlockCache should be somehow combined?  Dunno.
                
> 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
>
>         Attachments: CacheConfig92-v8.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13119903#comment-13119903 ] 

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


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


Tests run: 1018, Failures: 0, Errors: 0, Skipped: 21

woot

- Jonathan


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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13120582#comment-13120582 ] 

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


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

(Updated 2011-10-04 23:27:07.734296)


Review request for hbase, Dhruba Borthakur, Michael Stack, and Li Pi.


Changes
-------

Addresses Mikhail and Stack's review comments.

I've completely removed all external calls to the CacheConfig() constructor, always passing conf now.

Just kicked off a unit test run.


Summary
-------

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.

All tests are NOT passing yet, still working on it, but wanted to have something up today.  Basically "code complete" but broken :)


This addresses bug HBASE-4422.
    https://issues.apache.org/jira/browse/HBASE-4422


Diffs (updated)
-----

  /src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java PRE-CREATION 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/SimpleBlockCache.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1179008 

Diff: https://reviews.apache.org/r/2089/diff


Testing
-------

Still working through some tests that aren't passing.


Thanks,

Jonathan


                
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13124474#comment-13124474 ] 

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


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

(Updated 2011-10-10 21:08:15.751510)


Review request for hbase, Dhruba Borthakur, Michael Stack, and Li Pi.


Changes
-------

Fix to TestCacheOnWrite (not sure how this got through to now?)


Summary
-------

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.

All tests are NOT passing yet, still working on it, but wanted to have something up today.  Basically "code complete" but broken :)


This addresses bug HBASE-4422.
    https://issues.apache.org/jira/browse/HBASE-4422


Diffs (updated)
-----

  /src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java PRE-CREATION 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/SimpleBlockCache.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1179008 
  /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1179008 
  /src/test/java/org/apache/hadoop/hbase/util/TestMergeTool.java 1179008 

Diff: https://reviews.apache.org/r/2089/diff


Testing
-------

Still working through some tests that aren't passing.


Thanks,

Jonathan


                
> 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
>
>         Attachments: CacheConfig92-v8.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13120556#comment-13120556 ] 

jiraposter@reviews.apache.org 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13125588#comment-13125588 ] 

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



bq.  On 2011-10-12 02:24:55, Mikhail Bautin wrote:
bq.  > 1032 unit tests passed, 0 failed, 16 skipped (I still don't know why Maven skips tests and whether it is a problem -- any idea?)
bq.  > I think this patch is good to go.

Mikhail: My guess are these are the tests that have an '@Ignore' annotation preceding the '@Test' annotation.. though if I do the below:

$ grep -r '@Ignore' src/test/|grep -v svn|grep -v BROKE |wc

I get 23... maybe its something else?


- Michael


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


On 2011-10-11 22:18:32, 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-11 22:18:32)
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 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java PRE-CREATION 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/SimpleBlockCache.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1182021 
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
>
>         Attachments: CacheConfig92-v8.patch, HBASE-4422-FINAL-branch92.patch, HBASE-4422-FINAL-trunk.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13124485#comment-13124485 ] 

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


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

Ship it!


Looks good to me! Reviewed the StoreFile.java diff from the patch on the JIRA.

- Mikhail


On 2011-10-10 21:08:15, 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-10 21:08:15)
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 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java PRE-CREATION 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/SimpleBlockCache.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1179008 
bq.    /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1179008 
bq.    /src/test/java/org/apache/hadoop/hbase/util/TestMergeTool.java 1179008 
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
>
>         Attachments: CacheConfig92-v8.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13125410#comment-13125410 ] 

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


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


There still seem to be some ZK-related changes in the patch

-    this.conf.setLong("hbase.zookeeper.recoverable.waittime", 1000);
+    this.conf.setLong("zookeeper.recovery.retry.intervalmill", 100);
+    this.conf.setLong("zookeeper.recovery.retry", 1);

Also, for some reason the patch does not apply cleanly to StoreFile.java (the hunk that deletes getBlockCache), but that is easily fixed manually.

Debugging TestStoreFile and TestHFile...

- Mikhail


On 2011-10-11 20:35:51, 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-11 20:35:51)
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 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java PRE-CREATION 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/SimpleBlockCache.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1182021 
bq.    /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1182021 
bq.    /src/test/java/org/apache/hadoop/hbase/util/TestMergeTool.java 1182021 
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
>
>         Attachments: CacheConfig92-v8.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13125878#comment-13125878 ] 

Hudson commented on HBASE-4422:
-------------------------------

Integrated in HBase-0.92 #57 (See [https://builds.apache.org/job/HBase-0.92/57/])
    HBASE-4422  Move block cache parameters and references into single CacheConf class (jgray)

jgray : 
Files : 
* /hbase/branches/0.92/CHANGES.txt
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/SimpleBlockCache.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java
* /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java
* /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java

                
> 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
>
>         Attachments: CacheConfig92-v8.patch, HBASE-4422-FINAL-branch92.patch, HBASE-4422-FINAL-trunk.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Lars Hofhansl (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13117832#comment-13117832 ] 

Lars Hofhansl commented on HBASE-4422:
--------------------------------------

Want me to take your latest patch and fold my changes from HBASE-4496 into it?
I have a (manual) test to verify it.
                
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13125437#comment-13125437 ] 

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


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

(Updated 2011-10-11 22:18:32.037217)


Review request for hbase, Dhruba Borthakur, Michael Stack, and Li Pi.


Changes
-------

Lucky number 13!!!  This is the one!

Fixes failing TestHFile / TestStoreFile.  Thanks Mikhail.


Summary
-------

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.

All tests are NOT passing yet, still working on it, but wanted to have something up today.  Basically "code complete" but broken :)


This addresses bug HBASE-4422.
    https://issues.apache.org/jira/browse/HBASE-4422


Diffs (updated)
-----

  /src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java PRE-CREATION 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/SimpleBlockCache.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1182021 
  /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1182021 
  /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1182021 

Diff: https://reviews.apache.org/r/2089/diff


Testing
-------

Still working through some tests that aren't passing.


Thanks,

Jonathan


                
> 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
>
>         Attachments: CacheConfig92-v8.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13120501#comment-13120501 ] 

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


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


Sending the second part of comments. Still have not reviewed StoreFile changes. I guess my biggest concern is the initialization of a default CacheConfig in a lot of places (hopefully mostly tests) while it could be initialized based on the available Configuration.


/src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java
<https://reviews.apache.org/r/2089/#comment5336>

    Should not there be a way to create a default CacheConf() from a Configuration()? Currently it seems that you are creating a default CacheConfig to pass to getWriteFactory even though you have a conf available, potentially with some cache-related settings. Maybe there should be an overload of HFile.getWriterFactory that takes a conf and creates a default CacheConfig out of it.



/src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java
<https://reviews.apache.org/r/2089/#comment5337>

    The same concern about default CacheConfig as above.



/src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java
<https://reviews.apache.org/r/2089/#comment5338>

    nit: trailing whitespace (we probably should remove all of it in one patch someday :)



/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java
<https://reviews.apache.org/r/2089/#comment5340>

    Do you want to keep this debug code around?



/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java
<https://reviews.apache.org/r/2089/#comment5339>

    Is this debug code?



/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java
<https://reviews.apache.org/r/2089/#comment5341>

    Should the CacheConfig be initialized based on the configuration? (the same as other places)



/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java
<https://reviews.apache.org/r/2089/#comment5343>

    The same thing with CacheConfig and conf.



/src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java
<https://reviews.apache.org/r/2089/#comment5344>

    The same thing with CacheConfig and conf.



/src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java
<https://reviews.apache.org/r/2089/#comment5345>

    The same thing with CacheConfig and conf (OK, I won't mention this anymore).



/src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java
<https://reviews.apache.org/r/2089/#comment5348>

    Should not we move the cast to initialization and change blockCache's type to LruBlockCache?
    
    Is this method only intended for testing, by the way? If so, should this be indicated in the method name?


- Mikhail


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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13120543#comment-13120543 ] 

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



bq.  On 2011-10-04 21:28:58, Mikhail Bautin wrote:
bq.  > /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java, line 170
bq.  > <https://reviews.apache.org/r/2089/diff/5/?file=47785#file47785line170>
bq.  >
bq.  >     is .toString() necessary?

removed


bq.  On 2011-10-04 21:28:58, Mikhail Bautin wrote:
bq.  > /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java, line 35
bq.  > <https://reviews.apache.org/r/2089/diff/5/?file=47787#file47787line35>
bq.  >
bq.  >     Why not make this private?

done


bq.  On 2011-10-04 21:28:58, Mikhail Bautin wrote:
bq.  > /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java, line 67
bq.  > <https://reviews.apache.org/r/2089/diff/5/?file=47787#file47787line67>
bq.  >
bq.  >     Should this be suffixed with _KEY for consistency with other conf key constants in this class?

done


bq.  On 2011-10-04 21:28:58, Mikhail Bautin wrote:
bq.  > /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java, line 169
bq.  > <https://reviews.apache.org/r/2089/diff/5/?file=47787#file47787line169>
bq.  >
bq.  >     Is this constructor ever used outside of CacheConfig itself? It looks like a potential source of errors, because the caller has to get the order of 7 boolean arguments right.

it actually has to be used if you want to manually set stuff, so it does get used inside of some unit tests...

It's actually only used from within the package so I will make it pkg private


bq.  On 2011-10-04 21:28:58, Mikhail Bautin wrote:
bq.  > /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java, line 268
bq.  > <https://reviews.apache.org/r/2089/diff/5/?file=47787#file47787line268>
bq.  >
bq.  >     Why is this not private?

There is a RandomSeek unit test that injects a block cache.  This is not a change in behavior (moved over from how it was in StoreFile previously)


bq.  On 2011-10-04 21:28:58, Mikhail Bautin wrote:
bq.  > /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java, line 307
bq.  > <https://reviews.apache.org/r/2089/diff/5/?file=47787#file47787line307>
bq.  >
bq.  >     nit: space between "if" and the parenthesis :)

yeah i do that a lot :)  changed... thanks for the detailed look!


bq.  On 2011-10-04 21:28:58, Mikhail Bautin wrote:
bq.  > /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java, line 355
bq.  > <https://reviews.apache.org/r/2089/diff/5/?file=47790#file47790line355>
bq.  >
bq.  >     Will this always return false if there is no block cache?

yes.  all of the cacheConf calls do:

  public boolean shouldEvictOnClose() {
    return isBlockCacheEnabled() && this.evictOnClose;
  }


bq.  On 2011-10-04 21:28:58, Mikhail Bautin wrote:
bq.  > /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java, line 244
bq.  > <https://reviews.apache.org/r/2089/diff/5/?file=47791#file47791line244>
bq.  >
bq.  >     cacheBlock should probably be set to false if !cacheBlock.isBlockCacheEnabled()

same thing as above.  shouldCacheDataOnRead() is false if block cache is not enabled.


bq.  On 2011-10-04 21:28:58, Mikhail Bautin wrote:
bq.  > /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java, line 265
bq.  > <https://reviews.apache.org/r/2089/diff/5/?file=47791#file47791line265>
bq.  >
bq.  >     cacheBlock is not set to false if !cacheConf.isBlockCacheEnabled(). Could this result in an NPE?

same as above.  no check from cacheConf ever returns true if block cache is globally disabled


bq.  On 2011-10-04 21:28:58, Mikhail Bautin wrote:
bq.  > /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java, line 168
bq.  > <https://reviews.apache.org/r/2089/diff/5/?file=47793#file47793line168>
bq.  >
bq.  >     It would be good for readability and maybe for performance to assign cacheConf.shouldCacheIndexesOnWrite() to a local variable.

done


- Jonathan


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


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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13117458#comment-13117458 ] 

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



bq.  On 2011-09-29 01:10:28, Lars Hofhansl wrote:
bq.  > Cool... This makes it much simpler in the future.
bq.  > Are you still planning to fold HBASE-4496 into this?

Yes, I will look at that today.  I'm thinking about a new test for all this stuff since this diff is riddled with bugs and it's a bit random which tests fail :)


bq.  On 2011-09-29 01:10:28, Lars Hofhansl wrote:
bq.  > /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java, line 244
bq.  > <https://reviews.apache.org/r/2089/diff/1/?file=46310#file46310line244>
bq.  >
bq.  >     Hmm... The new code does not honor the cacheBlock flag that was passed into the method.

Yeah, this is the stuff I need to go back and review.  Thanks for pointing me to the right place :)

Will have a new diff up in a bit.


- Jonathan


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


On 2011-09-28 19:56:14, 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-09-28 19:56:14)
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 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1177030 
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 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1177030 
bq.    /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1177030 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1177030 
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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13125614#comment-13125614 ] 

Hudson commented on HBASE-4422:
-------------------------------

Integrated in HBase-TRUNK #2319 (See [https://builds.apache.org/job/HBase-TRUNK/2319/])
    HBASE-4422  Move block cache parameters and references into single CacheConf class (jgray)

jgray : 
Files : 
* /hbase/trunk/CHANGES.txt
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/DoubleBlockCache.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/SimpleBlockCache.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SingleSizeCache.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/slab/SlabCache.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java

                
> 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
>
>         Attachments: CacheConfig92-v8.patch, HBASE-4422-FINAL-branch92.patch, HBASE-4422-FINAL-trunk.patch
>
>
> 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

        

[jira] [Updated] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Phabricator (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Phabricator updated HBASE-4422:
-------------------------------

    Attachment: D1341.1.patch

mbautin requested code review of "[jira] [HBASE-4422] [89-fb] Move block cache parameters and references into single CacheConfig class".
Reviewers: Kannan, Karthik, nspiegelberg, Liyin, JIRA

  This is an 89-fb port of Jonathan Gray's CacheConfig patch. This diff also undoes Liyin's work to disable cache-on-write on compactions (rHBASEEIGHTNINEFBBRANCH1233731, the 89-fb version HBASE-3976). The status of HBASE-3976 is unclear, because it seems reverted in trunk and CacheConfig would completely change how it would work anyway. Thus, I am planning to investigate the status of HBASE-3976 in trunk add a unit test verifying whether we cache blocks on write in compactions (HBASE-5230), and implement HBASE-3976 in a consistent way in both trunk and 89-fb.

  Coming back to HBASE-4422: here is Jonathan's original description of CacheConfig. From StoreFile down to HFile, we previously used a boolean argument for each of the various block cache configuration parameters. The number of parameters is going to continue to increase as we look at compressed cache, etc. Every new config currently requires changing many constructors because it introduces a new boolean. In this patch we move everything into a single class so that modifications are much less disruptive.

TEST PLAN
  Unit tests, dev cluster, dark launch

REVISION DETAIL
  https://reviews.facebook.net/D1341

AFFECTED FILES
  src/main/java/org/apache/hadoop/hbase/HConstants.java
  src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java
  src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java
  src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java
  src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java
  src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
  src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java
  src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java
  src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java
  src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
  src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java
  src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
  src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
  src/main/java/org/apache/hadoop/hbase/io/hfile/SimpleBlockCache.java
  src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java
  src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
  src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
  src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
  src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
  src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
  src/main/java/org/apache/hadoop/hbase/regionserver/metrics/SchemaMetrics.java
  src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java
  src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java
  src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java
  src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java
  src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java
  src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java
  src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java
  src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java
  src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java
  src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java
  src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java
  src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java
  src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java
  src/test/java/org/apache/hadoop/hbase/io/hfile/TestScannerSelectionUsingTTL.java
  src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java
  src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java
  src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
  src/test/java/org/apache/hadoop/hbase/regionserver/CreateRandomStoreFile.java
  src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.java
  src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java
  src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java
  src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java
  src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java
  src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java
  src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
  src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java

MANAGE HERALD DIFFERENTIAL RULES
  https://reviews.facebook.net/herald/view/differential/

WHY DID I GET THIS EMAIL?
  https://reviews.facebook.net/herald/transcript/2805/

Tip: use the X-Herald-Rules header to filter Herald messages in your client.

                
> 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
>
>         Attachments: CacheConfig92-v8.patch, D1341.1.patch, HBASE-4422-FINAL-branch92.patch, HBASE-4422-FINAL-trunk.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Jonathan Gray (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13120637#comment-13120637 ] 

Jonathan Gray commented on HBASE-4422:
--------------------------------------

Yeah, if this goes to trunk but not 92 then begins the fun of rebasing patches for each because it changes so many constructors in/around HFile.
                
> 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
>
>         Attachments: CacheConfig92-v8.patch
>
>
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "Mikhail Bautin (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13119479#comment-13119479 ] 

Mikhail Bautin commented on HBASE-4422:
---------------------------------------

For some reason the most recent version of the patch does not apply for me (one hunk gets rejected in StoreFile.java). The way I am trying to apply the patch is

curl https://reviews.apache.org/r/2089/diff/4/raw/ | patch -p0

and I get http://pastebin.com/8aZ5r9Pf. My current revision is 1176613.

                
> 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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13119677#comment-13119677 ] 

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



bq.  On 2011-10-03 20:39:25, Lars Hofhansl wrote:
bq.  > /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java, line 64
bq.  > <https://reviews.apache.org/r/2089/diff/4/?file=47479#file47479line64>
bq.  >
bq.  >     That'd be pretty awesome :)

I have it implemented and tested on an internal branch.  It's not too bad once CacheConfig is in.

Also, some pretty awesome Delta Encoding stuff should be coming in the pipeline soon!  Can't wait....


- Jonathan


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


On 2011-10-03 18:19:34, 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-03 18:19:34)
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/util/BloomFilterFactory.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1178474 
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 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1178474 
bq.    /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1178474 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1178474 
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

        

[jira] [Commented] (HBASE-4422) Move block cache parameters and references into single CacheConf class

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13117801#comment-13117801 ] 

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


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

(Updated 2011-09-30 01:05:40.429350)


Review request for hbase, Dhruba Borthakur, Michael Stack, and Li Pi.


Changes
-------

Fixed bugs mentioned by Lars and a few others.  Lots of tests are passing (TestCacheOnWrite for example).  Running test suite now.  Also added a small test in TestBlocksRead that I was attempting to test for HBASE-4496 but it's passing so still need to dig on what exactly is wrong there.


Summary
-------

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.

All tests are NOT passing yet, still working on it, but wanted to have something up today.  Basically "code complete" but broken :)


This addresses bug HBASE-4422.
    https://issues.apache.org/jira/browse/HBASE-4422


Diffs (updated)
-----

  /src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java 1177471 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java 1177471 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java 1177471 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java PRE-CREATION 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java 1177471 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java 1177471 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java 1177471 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java 1177471 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java 1177471 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java 1177471 
  /src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java 1177471 
  /src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 1177471 
  /src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1177471 
  /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1177471 
  /src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1177471 
  /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1177471 
  /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 1177471 
  /src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java 1177471 
  /src/main/java/org/apache/hadoop/hbase/util/CompressionTest.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/HFilePerformanceEvaluation.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/RandomSeek.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFile.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFilePerformance.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileSeek.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileWriterV2.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestReseekTo.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileBlockCacheSummary.java 1177471 
  /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 1177471 

Diff: https://reviews.apache.org/r/2089/diff


Testing
-------

Still working through some tests that aren't passing.


Thanks,

Jonathan


                
> 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