You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "Mikhail Bautin (Created) (JIRA)" <ji...@apache.org> on 2012/02/22 04:24:57 UTC
[jira] [Created] (HBASE-5442) Use builder pattern in StoreFile and
HFile
Use builder pattern in StoreFile and HFile
------------------------------------------
Key: HBASE-5442
URL: https://issues.apache.org/jira/browse/HBASE-5442
Project: HBase
Issue Type: Improvement
Reporter: Mikhail Bautin
Assignee: Mikhail Bautin
We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
{code:java}
HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
.setParameter1(value1)
.setParameter2(value2)
...
.build();
{code}
Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile
and HFile
Posted by "Phabricator (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13216044#comment-13216044 ]
Phabricator commented on HBASE-5442:
------------------------------------
mbautin has commented on the revision "[jira] [HBASE-5442] [89-fb] Use builder pattern in StoreFile and HFile".
This passed all unit tests.
REVISION DETAIL
https://reviews.facebook.net/D1941
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Fix For: 0.94.0
>
> Attachments: D1893.1.patch, D1893.2.patch, D1941.1.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile and
HFile
Posted by "Phabricator (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Phabricator updated HBASE-5442:
-------------------------------
Attachment: D1893.1.patch
mbautin requested code review of "[jira] [HBASE-5442] Use builder pattern in StoreFile and HFile".
Reviewers: JIRA, stack, tedyu, Kannan, Karthik, Liyin
Cleaning up the factory method explosion in HFile writer and StoreFile. Now, adding a new parameter to HFile/StoreFile writer initialization will not require modifying constructor invocations all over the codebase.
TEST PLAN
Run unit tests
Deploy to dev cluster and run a load test
REVISION DETAIL
https://reviews.facebook.net/D1893
AFFECTED FILES
src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java
src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.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/mapreduce/HFileOutputFormat.java
src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.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/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/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/TestSeekTo.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/TestCompoundBloomFilter.java
src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java
src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionServerBulkLoad.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/4017/
Tip: use the X-Herald-Rules header to filter Herald messages in your client.
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Attachments: D1893.1.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile
and HFile
Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13216320#comment-13216320 ]
Hudson commented on HBASE-5442:
-------------------------------
Integrated in HBase-TRUNK-security #122 (See [https://builds.apache.org/job/HBase-TRUNK-security/122/])
[jira] [HBASE-5442] Use builder pattern in StoreFile and HFile
Summary: Cleaning up the factory method explosion in HFile writer and StoreFile.
Now, adding a new parameter to HFile/StoreFile writer initialization will not
require modifying factory method invocations all over the codebase.
Test Plan:
Run unit tests
Deploy to dev cluster and run a load test
Reviewers: JIRA, stack, tedyu, Kannan, Karthik, Liyin
Reviewed By: stack
Differential Revision: https://reviews.facebook.net/D1893 (Revision 1293095)
Result = FAILURE
mbautin :
Files :
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.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/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/Store.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.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/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/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/TestLoadIncrementalHFiles.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/CreateRandomStoreFile.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.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/TestHRegionServerBulkLoad.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/wal/TestWALReplay.java
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Fix For: 0.94.0
>
> Attachments: D1893.1.patch, D1893.2.patch, D1941.1.patch, D1941.2.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile and
HFile
Posted by "Mikhail Bautin (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mikhail Bautin updated HBASE-5442:
----------------------------------
Attachment: HFile-StoreFile-builder-2012-02-22_22_49_00.patch
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Attachments: D1893.1.patch, D1893.2.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile and
HFile
Posted by "Mikhail Bautin (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mikhail Bautin updated HBASE-5442:
----------------------------------
Status: Patch Available (was: Open)
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Attachments: D1893.1.patch, D1893.2.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile
and HFile
Posted by "Hadoop QA (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13215094#comment-13215094 ]
Hadoop QA commented on HBASE-5442:
----------------------------------
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12515717/HFile-StoreFile-builder-2012-02-22_22_49_00.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 73 new or modified tests.
-1 javadoc. The javadoc tool appears to have generated -135 warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
-1 findbugs. The patch appears to introduce 153 new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
-1 core tests. The patch failed these unit tests:
org.apache.hadoop.hbase.replication.TestReplicationPeer
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1029//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1029//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1029//console
This message is automatically generated.
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Attachments: D1893.1.patch, D1893.2.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile
and HFile
Posted by "Hadoop QA (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13214425#comment-13214425 ]
Hadoop QA commented on HBASE-5442:
----------------------------------
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12515717/HFile-StoreFile-builder-2012-02-22_22_49_00.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 73 new or modified tests.
-1 javadoc. The javadoc tool appears to have generated -135 warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
-1 findbugs. The patch appears to introduce 153 new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
-1 core tests. The patch failed these unit tests:
org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat
org.apache.hadoop.hbase.mapred.TestTableMapReduce
org.apache.hadoop.hbase.mapreduce.TestImportTsv
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1021//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1021//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1021//console
This message is automatically generated.
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Attachments: D1893.1.patch, D1893.2.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile
and HFile
Posted by "Phabricator (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13215419#comment-13215419 ]
Phabricator commented on HBASE-5442:
------------------------------------
mbautin has committed the revision "[jira] [HBASE-5442] Use builder pattern in StoreFile and HFile".
REVISION DETAIL
https://reviews.facebook.net/D1893
COMMIT
https://reviews.facebook.net/rHBASE1293095
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Attachments: D1893.1.patch, D1893.2.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile and
HFile
Posted by "Phabricator (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Phabricator updated HBASE-5442:
-------------------------------
Attachment: D1893.2.patch
mbautin updated the revision "[jira] [HBASE-5442] Use builder pattern in StoreFile and HFile".
Reviewers: JIRA, stack, tedyu, Kannan, Karthik, Liyin
Removing remaining StoreFile factory methods and replacing their invocations with StoreFile.WriterBuilder calls. Also, reverting the comparator type change in HFile writer API.
REVISION DETAIL
https://reviews.facebook.net/D1893
AFFECTED FILES
src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.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/mapreduce/HFileOutputFormat.java
src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.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/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/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/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/TestSeekTo.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/TestCompoundBloomFilter.java
src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java
src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionServerBulkLoad.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
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Attachments: D1893.1.patch, D1893.2.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile and
HFile
Posted by "Mikhail Bautin (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mikhail Bautin updated HBASE-5442:
----------------------------------
Resolution: Fixed
Fix Version/s: 0.94.0
Status: Resolved (was: Patch Available)
Committed to trunk.
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Fix For: 0.94.0
>
> Attachments: D1893.1.patch, D1893.2.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile and
HFile
Posted by "Phabricator (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Phabricator updated HBASE-5442:
-------------------------------
Attachment: D1941.4.patch
mbautin updated the revision "[jira] [HBASE-5442] [89-fb] Use builder pattern in StoreFile and HFile".
Reviewers: JIRA, khemani, Kannan, Liyin, Karthik, nspiegelberg
Removing unused imports in HBaseTestingUtility
REVISION DETAIL
https://reviews.facebook.net/D1941
AFFECTED FILES
src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.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/mapreduce/HFileOutputFormat.java
src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.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/util/CompressionTest.java
src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.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/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/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/TestSeekTo.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/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
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Fix For: 0.94.0
>
> Attachments: D1893.1.patch, D1893.2.patch, D1941.1.patch, D1941.2.patch, D1941.3.patch, D1941.4.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile and
HFile
Posted by "Phabricator (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Phabricator updated HBASE-5442:
-------------------------------
Attachment: D1983.1.patch
mbautin requested code review of "[jira] [HBASE-5442] Follow-up: fix NPE in StoreFile".
Reviewers: Liyin, JIRA
This probably sneaked in during rebasing somehow, because I did not catch this in my testing. If we pass null as data block encoder to a StoreFile writer builder, we still want to use a "no-op" data block encoder. This fixes the following unit tests: TestStore TestStoreFile TestFSErrorsExposed TestCompoundBloomFilter TestHLog
TEST PLAN
Run unit tests
REVISION DETAIL
https://reviews.facebook.net/D1983
AFFECTED FILES
src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
MANAGE HERALD DIFFERENTIAL RULES
https://reviews.facebook.net/herald/view/differential/
WHY DID I GET THIS EMAIL?
https://reviews.facebook.net/herald/transcript/4233/
Tip: use the X-Herald-Rules header to filter Herald messages in your client.
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Fix For: 0.94.0
>
> Attachments: D1893.1.patch, D1893.2.patch, D1941.1.patch, D1941.2.patch, D1941.3.patch, D1941.4.patch, D1941.5.patch, D1983.1.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile
and HFile
Posted by "Phabricator (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13216776#comment-13216776 ]
Phabricator commented on HBASE-5442:
------------------------------------
Kannan has accepted the revision "[jira] [HBASE-5442] [89-fb] Use builder pattern in StoreFile and HFile".
looks great!
REVISION DETAIL
https://reviews.facebook.net/D1941
BRANCH
hfile_builder10
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Fix For: 0.94.0
>
> Attachments: D1893.1.patch, D1893.2.patch, D1941.1.patch, D1941.2.patch, D1941.3.patch, D1941.4.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile
and HFile
Posted by "Phabricator (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13215281#comment-13215281 ]
Phabricator commented on HBASE-5442:
------------------------------------
stack has accepted the revision "[jira] [HBASE-5442] Use builder pattern in StoreFile and HFile".
+1
REVISION DETAIL
https://reviews.facebook.net/D1893
BRANCH
hfile_builder5
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Attachments: D1893.1.patch, D1893.2.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile
and HFile
Posted by "Phabricator (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13216209#comment-13216209 ]
Phabricator commented on HBASE-5442:
------------------------------------
Kannan has commented on the revision "[jira] [HBASE-5442] [89-fb] Use builder pattern in StoreFile and HFile".
super! one comment...
INLINE COMMENTS
src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java:594 we should add a withFavoredNodes() method too, to WriterBuilder.
Separtely, we need to figure out why we don't have any createWriter() calls that actually pass in a non-NULL favoredNodes list. Looks like something got lost in one of the previous commits.
REVISION DETAIL
https://reviews.facebook.net/D1941
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Fix For: 0.94.0
>
> Attachments: D1893.1.patch, D1893.2.patch, D1941.1.patch, D1941.2.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile
and HFile
Posted by "Phabricator (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13217700#comment-13217700 ]
Phabricator commented on HBASE-5442:
------------------------------------
mbautin has committed the revision "[jira] [HBASE-5442] [89-fb] Use builder pattern in StoreFile and HFile".
REVISION DETAIL
https://reviews.facebook.net/D1941
COMMIT
https://reviews.facebook.net/rHBASEEIGHTNINEFBBRANCH1294395
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Fix For: 0.94.0
>
> Attachments: D1893.1.patch, D1893.2.patch, D1941.1.patch, D1941.2.patch, D1941.3.patch, D1941.4.patch, D1941.5.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile and
HFile
Posted by "Phabricator (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Phabricator updated HBASE-5442:
-------------------------------
Attachment: D1941.1.patch
mbautin requested code review of "[jira] [HBASE-5442] [89-fb] Use builder pattern in StoreFile and HFile".
Reviewers: JIRA, khemani, Kannan
Cleaning up the factory method explosion in HFile writer and StoreFile. Now, adding a new parameter to HFile/StoreFile writer initialization will not require modifying factory method invocations all over the codebase.
HFile writer parametersin 89-fb that are not there in trunk are: favoredNodes, bloom error rate (customized for the column family), bits per checksum.
This patch also has a bit more cleanup than the trunk version (https://reviews.facebook.net/D1893), getting rid of a couple more annoying constructors. This will be contributed to trunk as a follow-up patch
TEST PLAN
Unit tests, deploy to dev cluster
REVISION DETAIL
https://reviews.facebook.net/D1941
AFFECTED FILES
src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.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/mapreduce/HFileOutputFormat.java
src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.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/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/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/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/TestSeekTo.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/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/4107/
Tip: use the X-Herald-Rules header to filter Herald messages in your client.
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Fix For: 0.94.0
>
> Attachments: D1893.1.patch, D1893.2.patch, D1941.1.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile
and HFile
Posted by "Phabricator (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13215280#comment-13215280 ]
Phabricator commented on HBASE-5442:
------------------------------------
mbautin has commented on the revision "[jira] [HBASE-5442] Use builder pattern in StoreFile and HFile".
@stack: @tedyu: please accept this if it looks OK to commit
REVISION DETAIL
https://reviews.facebook.net/D1893
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Attachments: D1893.1.patch, D1893.2.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile
and HFile
Posted by "Mikhail Bautin (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13215278#comment-13215278 ]
Mikhail Bautin commented on HBASE-5442:
---------------------------------------
Re-ran failed unit tests locally:
Running org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat
Tests run: 9, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 212.073 sec
Running org.apache.hadoop.hbase.mapreduce.TestImportTsv
Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 79.808 sec
Running org.apache.hadoop.hbase.mapreduce.TestTableMapReduce
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 98.041 sec
Running org.apache.hadoop.hbase.mapred.TestTableMapReduce
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 65.51 sec
Running org.apache.hadoop.hbase.replication.TestReplicationPeer
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 7.776 sec
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Attachments: D1893.1.patch, D1893.2.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile and
HFile
Posted by "Phabricator (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Phabricator updated HBASE-5442:
-------------------------------
Attachment: D1941.3.patch
mbautin updated the revision "[jira] [HBASE-5442] [89-fb] Use builder pattern in StoreFile and HFile".
Reviewers: JIRA, khemani, Kannan, Liyin, Karthik, nspiegelberg
Addressing Kannan's comment. Putting withFavoredNodes and getFavoredNodes back in. The getFavoredNode invocation was removed in https://reviews.facebook.net/rHBASEEIGHTNINEFBBRANCH1239905#fcf5a7a4 by mistake.
REVISION DETAIL
https://reviews.facebook.net/D1941
AFFECTED FILES
src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.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/mapreduce/HFileOutputFormat.java
src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.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/util/CompressionTest.java
src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.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/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/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/TestSeekTo.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/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
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Fix For: 0.94.0
>
> Attachments: D1893.1.patch, D1893.2.patch, D1941.1.patch, D1941.2.patch, D1941.3.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile
and HFile
Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13216336#comment-13216336 ]
Hudson commented on HBASE-5442:
-------------------------------
Integrated in HBase-TRUNK #2669 (See [https://builds.apache.org/job/HBase-TRUNK/2669/])
[jira] [HBASE-5442] Use builder pattern in StoreFile and HFile
Summary: Cleaning up the factory method explosion in HFile writer and StoreFile.
Now, adding a new parameter to HFile/StoreFile writer initialization will not
require modifying factory method invocations all over the codebase.
Test Plan:
Run unit tests
Deploy to dev cluster and run a load test
Reviewers: JIRA, stack, tedyu, Kannan, Karthik, Liyin
Reviewed By: stack
Differential Revision: https://reviews.facebook.net/D1893 (Revision 1293095)
Result = SUCCESS
mbautin :
Files :
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.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/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/Store.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.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/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/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/TestLoadIncrementalHFiles.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/CreateRandomStoreFile.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.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/TestHRegionServerBulkLoad.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/wal/TestWALReplay.java
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Fix For: 0.94.0
>
> Attachments: D1893.1.patch, D1893.2.patch, D1941.1.patch, D1941.2.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile
and HFile
Posted by "Phabricator (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13217764#comment-13217764 ]
Phabricator commented on HBASE-5442:
------------------------------------
mbautin has committed the revision "[jira] [HBASE-5442] Follow-up: fix NPE in StoreFile".
REVISION DETAIL
https://reviews.facebook.net/D1983
COMMIT
https://reviews.facebook.net/rHBASEEIGHTNINEFBBRANCH1294423
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Fix For: 0.94.0
>
> Attachments: D1893.1.patch, D1893.2.patch, D1941.1.patch, D1941.2.patch, D1941.3.patch, D1941.4.patch, D1941.5.patch, D1983.1.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile
and HFile
Posted by "stack (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13214868#comment-13214868 ]
stack commented on HBASE-5442:
------------------------------
@Mikhail Thats the usual set of three that fail on hadoopqa, fyi.
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Attachments: D1893.1.patch, D1893.2.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile
and HFile
Posted by "Phabricator (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13214322#comment-13214322 ]
Phabricator commented on HBASE-5442:
------------------------------------
stack has commented on the revision "[jira] [HBASE-5442] Use builder pattern in StoreFile and HFile".
+1 on commit if hadoopqa is cool w/ this patch
INLINE COMMENTS
src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java:112 You meant this Mikhail?
src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java:142 I like all this red
REVISION DETAIL
https://reviews.facebook.net/D1893
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Attachments: D1893.1.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile
and HFile
Posted by "Phabricator (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13217723#comment-13217723 ]
Phabricator commented on HBASE-5442:
------------------------------------
Liyin has accepted the revision "[jira] [HBASE-5442] Follow-up: fix NPE in StoreFile".
LGTM. Thanks Mikhail !
REVISION DETAIL
https://reviews.facebook.net/D1983
BRANCH
master
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Fix For: 0.94.0
>
> Attachments: D1893.1.patch, D1893.2.patch, D1941.1.patch, D1941.2.patch, D1941.3.patch, D1941.4.patch, D1941.5.patch, D1983.1.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile and
HFile
Posted by "Phabricator (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Phabricator updated HBASE-5442:
-------------------------------
Attachment: D1941.5.patch
mbautin updated the revision "[jira] [HBASE-5442] [89-fb] Use builder pattern in StoreFile and HFile".
Reviewers: JIRA, khemani, Kannan, Liyin, Karthik, nspiegelberg
Rebasing on the HColumnDescriptor refactoring
REVISION DETAIL
https://reviews.facebook.net/D1941
AFFECTED FILES
src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.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/mapreduce/HFileOutputFormat.java
src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.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/util/CompressionTest.java
src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.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/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/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/TestSeekTo.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/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
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Fix For: 0.94.0
>
> Attachments: D1893.1.patch, D1893.2.patch, D1941.1.patch, D1941.2.patch, D1941.3.patch, D1941.4.patch, D1941.5.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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-5442) Use builder pattern in StoreFile and
HFile
Posted by "Phabricator (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Phabricator updated HBASE-5442:
-------------------------------
Attachment: D1941.2.patch
mbautin updated the revision "[jira] [HBASE-5442] [89-fb] Use builder pattern in StoreFile and HFile".
Reviewers: JIRA, khemani, Kannan, Liyin, Karthik, nspiegelberg
Removing irrelevant javadoc from StoreFile writer constructor
REVISION DETAIL
https://reviews.facebook.net/D1941
AFFECTED FILES
src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.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/mapreduce/HFileOutputFormat.java
src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.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/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/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/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/TestSeekTo.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/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
> Use builder pattern in StoreFile and HFile
> ------------------------------------------
>
> Key: HBASE-5442
> URL: https://issues.apache.org/jira/browse/HBASE-5442
> Project: HBase
> Issue Type: Improvement
> Reporter: Mikhail Bautin
> Assignee: Mikhail Bautin
> Fix For: 0.94.0
>
> Attachments: D1893.1.patch, D1893.2.patch, D1941.1.patch, D1941.2.patch, HFile-StoreFile-builder-2012-02-22_22_49_00.patch
>
>
> We have five ways to create an HFile writer, two ways to create a StoreFile writer, and the sets of parameters keep changing, creating a lot of confusion, especially when porting patches across branches. The same thing is happening to HColumnDescriptor. I think we should move to a "builder pattern" solution, e.g.
> {code:java}
> HFileWriter w = HFile.getWriterBuilder(conf, <some common args>)
> .setParameter1(value1)
> .setParameter2(value2)
> ...
> .build();
> {code}
> Each parameter setter being on its own line will make merges/cherry-pick work properly, we will not have to even mention default parameters again, and we can eliminate a dozen impossible-to-remember constructors.
> This particular JIRA addresses StoreFile and HFile refactoring. For HColumnDescriptor refactoring see HBASE-5357.
--
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