Marking closed.

Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #399 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/399/)
HBASE-7603 refactor storefile management in HStore in order to support things like LevelDB-style compactions (Sergey) (Revision 1444262)

Result = FAILURE
tedyu :
Files :

• /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreFileManager.java
• /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
• /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
• /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
• /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
• /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileManager.java
• /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
• /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreUtils.java
• /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionPolicy.java
• /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/DefaultCompactionPolicy.java
• /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
• /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestScannerSelectionUsingTTL.java
• /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
• /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java
• /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
• /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java
• /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java

Integrated in HBase-TRUNK #3864 (See https://builds.apache.org/job/HBase-TRUNK/3864/)
HBASE-7603 refactor storefile management in HStore in order to support things like LevelDB-style compactions (Sergey) (Revision 1444262)

Result = FAILURE
tedyu :
Files :

• /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreFileManager.java
• /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
• /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
• /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
• /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
• /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileManager.java
• /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
• /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreUtils.java
• /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionPolicy.java
• /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/DefaultCompactionPolicy.java
• /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
• /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestScannerSelectionUsingTTL.java
• /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
• /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java
• /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
• /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java
• /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12568636/7603-v11.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 21 new or modified tests.

+1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 lineLengths. The patch does not introduce lines longer than 100

+1 core tests. The patch passed unit tests in .

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4386//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4386//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4386//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4386//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4386//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4386//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4386//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4386//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4386//console

This message is automatically generated.

Integrated to trunk.

Thanks for the patch, Sergey.

Thanks for the review, Jimmy and Matteo.

It looks good to me.

These tests pass locally

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12568608/HBASE-7603-v11.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 21 new or modified tests.

+1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 lineLengths. The patch does not introduce lines longer than 100

-1 core tests. The patch failed these unit tests:
org.apache.hadoop.hbase.security.access.TestAccessControlFilter
org.apache.hadoop.hbase.master.TestRollingRestart

-1 core zombie tests. There are 1 zombie test(s):

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4383//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4383//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4383//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4383//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4383//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4383//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4383//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4383//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4383//console

This message is automatically generated.

The latter is now in HBASE-7784

Hey Jimmy Xiang do you want to take a look?

The patch looks good to me, there're still stuff like requestCompaction(), needsCompaction(), getCompactPriority() that probably should have been moved to CompactionPolicy/Compactor during the HBASE-7516, but I guess we can move that in another jira.

feedback from /r/, removed all [level TODOs, they are being addressed in another JIRA anyway

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12568295/HBASE-7603-v10.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 21 new or modified tests.

+1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 lineLengths. The patch does not introduce lines longer than 100

+1 core tests. The patch passed unit tests in .

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4358//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4358//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4358//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4358//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4358//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4358//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4358//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4358//console

This message is automatically generated.

Feedback from /r/, I opened HBASE-7780 for unrelated cleanup of HStore.

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12568105/HBASE-7603-v9.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 21 new or modified tests.

+1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 lineLengths. The patch does not introduce lines longer than 100

+1 core tests. The patch passed unit tests in .

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4343//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4343//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4343//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4343//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4343//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4343//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4343//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4343//console

This message is automatically generated.

Another small refinement based on working on JIRAs based on this - added loadFiles; when this is pluggable I don't like the idea of doing heavy ops in ctor.

Oh, and stripe compaction algorithm is described in summary in HBASE-7667.

Matteo Bertozzi, Ted Yu - is there more feedback? I submitted example usage to HBASE-7679. There's writeup attached to HBASE-7519 that describes how it can be made pluggable, and usage patterns for level compactions that also apply to stripe compactions.
This issue blocks several other issues (making store file manager pluggable with compaction policy, and actually working on compaction algorithms.
Thanks!

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12567904/HBASE-7603-v8.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 21 new or modified tests.

+1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 lineLengths. The patch does not introduce lines longer than 100

+1 core tests. The patch passed unit tests in .

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4323//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4323//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4323//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4323//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4323//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4323//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4323//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4323//console

This message is automatically generated.

CR feedback from /r/

will fix long line together with next iteration of review...

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12567877/HBASE-7603-v7.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 18 new or modified tests.

+1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 lineLengths. The patch introduces lines longer than 100

+1 core tests. The patch passed unit tests in .

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4322//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4322//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4322//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4322//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4322//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4322//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4322//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4322//console

This message is automatically generated.

CR feedback, also moved some code from StoreFileManager to StoreUtils (finding largest file) and StoreFile (finding midpoint of a file)

And sorting in current codebase is gone:

setStoreFiles does the sorting
Rest - fixed

Saw the following which basically moves the sorting to the setter:

+  private void setStoreFiles(List<StoreFile> storeFiles) {
+    Collections.sort(storeFiles, StoreFile.Comparators.SEQ_ID);

It would be nice to name the above method: sortAndSetStoreFiles()

Review board lags latest patch.

+class DefaultStoreFileManager implements StoreFileManager {

Please add audience annotation.

+   * Adds new file, either for from MemStore flush of bulk insert, into the structure.
+   * @param sf New store file.
+   */
+  public abstract void insertNewFile(StoreFile sf);

I guess you wanted to say:

+   * Adds new file, either from MemStore flush or bulk insert, into the structure.

The implementation appends the StoreFile to the end of list:

+  public void insertNewFile(StoreFile sf) {
+    ArrayList<StoreFile> newFiles = new ArrayList<StoreFile>(storefiles);
+    newFiles.add(sf);

And sorting in current codebase is gone:

-      this.storefiles = sortAndClone(newFiles);
+      this.storeFiles.insertNewFile(sf);

Please explain.

Posted the patch there, and the updated patch here

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12567711/HBASE-7603-v6.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 18 new or modified tests.

+1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 lineLengths. The patch introduces lines longer than 100

+1 core tests. The patch passed unit tests in .

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4302//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4302//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4302//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4302//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4302//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4302//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4302//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4302//console

This message is automatically generated.

Ok, I will post stripe patch based on this into 7667... however, I don't think level should block stripe.

I don't want a super giant patch with all the possible store file manager specializations.
If you take a look at the snapshot development for example, you can see tons of jiras with small patches.
If you have a branch where you develop the stripe or level compaction (or both) that uses the store file manager, I think is better to consolidate the store file manager interface.
After that you can post 3 or more patch: Store File Manager, Level Compaction, Stripe Compaction (and since we've already merged the pluggable compaction I think that will be another refactor patch for the pluggable compaction interface).

Matteo Bertozzi I have a first cut of stripe implementation semi-ready, but it will be a large patch. I am hoping to have it ready tomorrow. It also doesn't include compaction policy for stripe yet.
Yes, the interface may change but only in details (e.g. like I generalized to collections now because flat lists do not make sense for everyone).
Why I don't want to put anything in one patch (and same for compactionpolicy comments)... if I were to put it all in one patch it would never get reviewed by anyone, same as HBASE-7055 and HBASE-7236 until they were split into many separate jiras.
When I refactored CompactionPolicy/ported CompactionConfiguration refactor/and then Jimmy made it pluggable, there were no code present with use cases for those... but they were done separately to make CRs easier.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12567438/HBASE-7603-v5.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 18 new or modified tests.

+1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 lineLengths. The patch introduces lines longer than 100

+1 core tests. The patch passed unit tests in .

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4278//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4278//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4278//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4278//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4278//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4278//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4278//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4278//console

This message is automatically generated.

I think that if you want to merge it now to trunk, you should just do a refactor to move the code out, and to have the store ready for the future.

Having comments like this:

// TODO: move this comment when implementing Level

// TODO [level compaction]: this will not work for level algo; need to move the decision into CompationPolicy

bring me to the conclusion that probably this refactor is not enough to be future proof, and probably it will change again for stripe compaction or others. So why we should merge this code now, with no real use cases?

Think about having a level/stripe compaction branch and merge everything once you've defined the right interface.

I really want to see this stuff going in as soon as possible, but I'll like to see something that uses it.

Addressed all TODO [level except 3 that directly relate to policy implementation and are out of the scope of this JIRA. Also generalized the interface to use Collection.

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12567276/HBASE-7603-v4.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 3 new or modified tests.

+1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 lineLengths. The patch does not introduce lines longer than 100

+1 core tests. The patch passed unit tests in .

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4267//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4267//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4267//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4267//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4267//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4267//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4267//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4267//console

This message is automatically generated.

Rebased the patch. I opened a bunch of separate jiras for other things related to stripe compactions... I was hoping to do the work e.g. additional pluggability to plug this in together with policy/compactor, in there jiras, to avoid huge patches. I can also do it here... please advise.

Hmm... these todos are better addressed at the time when implementing the compaction. If I address them all now the patch will be really big.
As for why the refactoring is needed, I will attach a patch (probably tomorrow) in the dependent JIRA for stripe compaction file management example.

The idea is good, but I think that you should address all the TODO before having the patch ready to commit.
Also the TODOs are related to the level compaction and if I remember correctly you've said in a jira that you want to start the stripe one instead of the level one.
Try to organize your thoughts about both compactions, what is needed to the store file manager, what are the extra metadata field and so on, and put them in a document. I think that having something that explains why we need to move out and refactor this code it will be nice, for the guys that are not looking directly at the code.

Hi. Ping?

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12566858/HBASE-7603-v3.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 3 new or modified tests.

+1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 lineLengths. The patch does not introduce lines longer than 100

+1 core tests. The patch passed unit tests in .

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4221//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4221//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4221//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4221//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4221//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4221//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4221//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4221//console

This message is automatically generated.

More CR feedback.

Achievement unlocked: +1 overall.

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12566830/HBASE-7603-v2.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 3 new or modified tests.

+1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 lineLengths. The patch does not introduce lines longer than 100

+1 core tests. The patch passed unit tests in .

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4217//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4217//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4217//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4217//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4217//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4217//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4217//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4217//console

This message is automatically generated.

Fix broken tests... bad morning

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12566807/HBASE-7603-v2.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 3 new or modified tests.

+1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

-1 findbugs. The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 lineLengths. The patch does not introduce lines longer than 100

-1 core tests. The patch failed these unit tests:
org.apache.hadoop.hbase.coprocessor.TestCoprocessorInterface
org.apache.hadoop.hbase.regionserver.TestMinVersions
org.apache.hadoop.hbase.regionserver.TestScanner
org.apache.hadoop.hbase.regionserver.TestKeepDeletes
org.apache.hadoop.hbase.regionserver.TestColumnSeeking

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4214//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4214//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4214//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4214//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4214//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4214//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4214//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4214//console

This message is automatically generated.

Addressed feedback from JIRA, removed lines longer than 100.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12566606/HBASE-7603-v1.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 3 new or modified tests.

+1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 lineLengths. The patch introduces lines longer than 100

+1 core tests. The patch passed unit tests in .

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4193//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4193//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4193//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4193//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4193//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4193//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4193//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4193//console

This message is automatically generated.

tiny change, wrong class used for logging

Note that the patch is first small step... I am proceeding in small steps from the outset given my previous experience with large patches

Jimmy Xiang Matteo Bertozzi Do you guys want to review this? Thanks!

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12565897/HBASE-7603-v0.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 3 new or modified tests.

+1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

-1 findbugs. The patch appears to introduce 3 new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 lineLengths. The patch introduces lines longer than 100

+1 core tests. The patch passed unit tests in .

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4122//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4122//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4122//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4122//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4122//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4122//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4122//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4122//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4122//console

This message is automatically generated.

Note that it says in the comment that it's not about FS layout (perhaps I should rename the class), it's about relations between storeFiles.
That way we can implement level db like storage where not all files are needed for reads, and files are not in one sequence.

I like the Storage File Manager approach. (Think about adding a createFileInTemp(), commitFile())
I think we should group all the fs operation in a single place, instead everything mixed in HRegion, Store, StoreFile, ...

on the other side we have a couple of patches out there that can conflict together. (snapshots, pluggable compaction, ...)
It's not something that should stop the work, but just to keep in mind, that there will be some extra work to merge them togheder.

Also on /r/ - https://reviews.apache.org/r/9051/

Here's a take at the patch. It has a lot of "TODO [level compaction]" added to where there are assumptions/usage of store files that will not work with it, especially around all things compaction, which need to be handled in HBASE-7516 or some other JIRA to make compactions fully pluggable.

Attached some notes in HBASE-7519