[HBASE-7603] refactor storefile management in HStore in order to support things like LevelDB-style compactions - ASF JIRA

Hide

Permalink

Sergey Shelukhin added a comment - 18/Jan/13 03:37

Attached some notes in HBASE-7519

Show

Sergey Shelukhin added a comment - 18/Jan/13 03:37 Attached some notes in HBASE-7519

Hide

Permalink

Sergey Shelukhin added a comment - 22/Jan/13 02:44

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.

Show

Sergey Shelukhin added a comment - 22/Jan/13 02:44 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.

Hide

Permalink

Sergey Shelukhin added a comment - 22/Jan/13 02:49

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

Show

Sergey Shelukhin added a comment - 22/Jan/13 02:49 Also on /r/ - https://reviews.apache.org/r/9051/

Hide

Permalink

Matteo Bertozzi added a comment - 22/Jan/13 02:57

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.

Show

Matteo Bertozzi added a comment - 22/Jan/13 02:57 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.

Hide

Permalink

Sergey Shelukhin added a comment - 22/Jan/13 03:04

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.

Show

Sergey Shelukhin added a comment - 22/Jan/13 03:04 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.

Hide

Permalink

Hadoop QA added a comment - 22/Jan/13 03:46

-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 .

This message is automatically generated.

Show

Hadoop QA added a comment - 22/Jan/13 03:46 -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.

Hide

Permalink

Sergey Shelukhin added a comment - 23/Jan/13 01:16

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

Show

Sergey Shelukhin added a comment - 23/Jan/13 01:16 Jimmy Xiang Matteo Bertozzi Do you guys want to review this? Thanks!

Hide

Permalink

Sergey Shelukhin added a comment - 23/Jan/13 01:18

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

Show

Sergey Shelukhin added a comment - 23/Jan/13 01:18 Note that the patch is first small step... I am proceeding in small steps from the outset given my previous experience with large patches

Hide

Permalink

Sergey Shelukhin added a comment - 26/Jan/13 03:33

tiny change, wrong class used for logging

Show

Sergey Shelukhin added a comment - 26/Jan/13 03:33 tiny change, wrong class used for logging

Hide

Permalink

Hadoop QA added a comment - 26/Jan/13 04:34

-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 .

This message is automatically generated.

Show

Hadoop QA added a comment - 26/Jan/13 04:34 -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.

Hide

Permalink

Sergey Shelukhin added a comment - 28/Jan/13 19:35

Addressed feedback from JIRA, removed lines longer than 100.

Show

Sergey Shelukhin added a comment - 28/Jan/13 19:35 Addressed feedback from JIRA, removed lines longer than 100.

Hide

Permalink

Hadoop QA added a comment - 28/Jan/13 19:58

-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

This message is automatically generated.

Show

Hadoop QA added a comment - 28/Jan/13 19:58 -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.

Hide

Permalink

Sergey Shelukhin added a comment - 28/Jan/13 21:46

Fix broken tests... bad morning

Show

Sergey Shelukhin added a comment - 28/Jan/13 21:46 Fix broken tests... bad morning

Hide

Permalink

Hadoop QA added a comment - 28/Jan/13 22:46

+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 .

This message is automatically generated.

Show

Hadoop QA added a comment - 28/Jan/13 22:46 +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.

Hide

Permalink

Sergey Shelukhin added a comment - 28/Jan/13 22:59

Achievement unlocked: +1 overall.

Show

Sergey Shelukhin added a comment - 28/Jan/13 22:59 Achievement unlocked: +1 overall.

Hide

Permalink

Sergey Shelukhin added a comment - 29/Jan/13 00:17

More CR feedback.

Show

Sergey Shelukhin added a comment - 29/Jan/13 00:17 More CR feedback.

Hide

Permalink

Hadoop QA added a comment - 29/Jan/13 01:19

+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 .

This message is automatically generated.

Show

Hadoop QA added a comment - 29/Jan/13 01:19 +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.

Hide

Permalink

Sergey Shelukhin added a comment - 30/Jan/13 18:53

Hi. Ping?

Show

Sergey Shelukhin added a comment - 30/Jan/13 18:53 Hi. Ping?

Hide

Permalink

Matteo Bertozzi added a comment - 30/Jan/13 19:04

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.

Show

Matteo Bertozzi added a comment - 30/Jan/13 19:04 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.

Hide

Permalink

Sergey Shelukhin added a comment - 30/Jan/13 20:53

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.

Show

Sergey Shelukhin added a comment - 30/Jan/13 20:53 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.

Hide

Permalink

Sergey Shelukhin added a comment - 31/Jan/13 01:35

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.

Show

Sergey Shelukhin added a comment - 31/Jan/13 01:35 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.

Hide

Permalink

Hadoop QA added a comment - 31/Jan/13 03:08

+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 .

This message is automatically generated.

Show

Hadoop QA added a comment - 31/Jan/13 03:08 +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.

Hide

Permalink

Sergey Shelukhin added a comment - 31/Jan/13 21:41

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.

Show

Sergey Shelukhin added a comment - 31/Jan/13 21:41 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.

Hide

Permalink

Matteo Bertozzi added a comment - 31/Jan/13 23:17

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.

Show

Matteo Bertozzi added a comment - 31/Jan/13 23:17 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.

Hide

Permalink

Hadoop QA added a comment - 31/Jan/13 23:30

-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 .

This message is automatically generated.

Show

Hadoop QA added a comment - 31/Jan/13 23:30 -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.

Hide

Permalink

Sergey Shelukhin added a comment - 31/Jan/13 23:57

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.

Show

Sergey Shelukhin added a comment - 31/Jan/13 23:57 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.

Hide

Permalink

Matteo Bertozzi added a comment - 01/Feb/13 00:12

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).

Show

Matteo Bertozzi added a comment - 01/Feb/13 00:12 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).

Hide

Permalink

Sergey Shelukhin added a comment - 01/Feb/13 00:18

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

Show

Sergey Shelukhin added a comment - 01/Feb/13 00:18 Ok, I will post stripe patch based on this into 7667... however, I don't think level should block stripe.

Hide

Permalink

Hadoop QA added a comment - 02/Feb/13 06:47

-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 .

This message is automatically generated.

Show

Hadoop QA added a comment - 02/Feb/13 06:47 -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.

Hide

Permalink

Sergey Shelukhin added a comment - 04/Feb/13 17:44

Posted the patch there, and the updated patch here

Show

Sergey Shelukhin added a comment - 04/Feb/13 17:44 Posted the patch there, and the updated patch here

Hide

Permalink

Ted Yu added a comment - 04/Feb/13 18:45

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.

Show

Ted Yu added a comment - 04/Feb/13 18:45 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.

Hide

Permalink

Ted Yu added a comment - 04/Feb/13 18:52

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()

Show

Ted Yu added a comment - 04/Feb/13 18:52 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()

Hide

Permalink

Sergey Shelukhin added a comment - 04/Feb/13 19:43

And sorting in current codebase is gone:

setStoreFiles does the sorting
Rest - fixed

Show

Sergey Shelukhin added a comment - 04/Feb/13 19:43 And sorting in current codebase is gone: setStoreFiles does the sorting Rest - fixed

Hide

Permalink

Sergey Shelukhin added a comment - 04/Feb/13 19:50

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

Show

Sergey Shelukhin added a comment - 04/Feb/13 19:50 CR feedback, also moved some code from StoreFileManager to StoreUtils (finding largest file) and StoreFile (finding midpoint of a file)

Hide

Permalink

Hadoop QA added a comment - 04/Feb/13 20:47

-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 .

This message is automatically generated.

Show

Hadoop QA added a comment - 04/Feb/13 20:47 -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.

Hide

Permalink

Sergey Shelukhin added a comment - 04/Feb/13 21:28

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

Show

Sergey Shelukhin added a comment - 04/Feb/13 21:28 will fix long line together with next iteration of review...

Hide

Permalink

Sergey Shelukhin added a comment - 04/Feb/13 23:21

CR feedback from /r/

Show

Sergey Shelukhin added a comment - 04/Feb/13 23:21 CR feedback from /r/

Hide

Permalink

Hadoop QA added a comment - 05/Feb/13 00:25

+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 .

This message is automatically generated.

Show

Hadoop QA added a comment - 05/Feb/13 00:25 +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.

Hide

Permalink

Sergey Shelukhin added a comment - 05/Feb/13 19:37

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!

Show

Sergey Shelukhin added a comment - 05/Feb/13 19:37 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!

Hide

Permalink

Sergey Shelukhin added a comment - 05/Feb/13 19:39

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

Show

Sergey Shelukhin added a comment - 05/Feb/13 19:39 Oh, and stripe compaction algorithm is described in summary in HBASE-7667 .

Hide

Permalink

Sergey Shelukhin added a comment - 05/Feb/13 22:21

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.

Show

Sergey Shelukhin added a comment - 05/Feb/13 22:21 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.

Hide

Permalink

Hadoop QA added a comment - 05/Feb/13 23:24

+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 .

This message is automatically generated.

Show

Hadoop QA added a comment - 05/Feb/13 23:24 +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.

Hide

Permalink

Sergey Shelukhin added a comment - 06/Feb/13 20:38

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

Show

Sergey Shelukhin added a comment - 06/Feb/13 20:38 Feedback from /r/, I opened HBASE-7780 for unrelated cleanup of HStore.

Hide

Permalink

Hadoop QA added a comment - 06/Feb/13 21:38

+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 .

This message is automatically generated.

Show

Hadoop QA added a comment - 06/Feb/13 21:38 +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.

Hide

Permalink

Sergey Shelukhin added a comment - 08/Feb/13 18:08

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

Show

Sergey Shelukhin added a comment - 08/Feb/13 18:08 feedback from /r/, removed all [level TODOs, they are being addressed in another JIRA anyway

Hide

Permalink

Matteo Bertozzi added a comment - 08/Feb/13 18:19

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.

Show

Matteo Bertozzi added a comment - 08/Feb/13 18:19 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.

Hide

Permalink

Sergey Shelukhin added a comment - 08/Feb/13 18:22

The latter is now in ~~HBASE-7784~~

Show

Sergey Shelukhin added a comment - 08/Feb/13 18:22 The latter is now in HBASE-7784

Hide

Permalink

Hadoop QA added a comment - 08/Feb/13 19:14

-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):

This message is automatically generated.

Show

Hadoop QA added a comment - 08/Feb/13 19:14 -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.

Hide

Permalink

Sergey Shelukhin added a comment - 08/Feb/13 19:42

These tests pass locally

Show

Sergey Shelukhin added a comment - 08/Feb/13 19:42 These tests pass locally

Hide

Permalink

Jimmy Xiang added a comment - 08/Feb/13 21:26

It looks good to me.

Show

Jimmy Xiang added a comment - 08/Feb/13 21:26 It looks good to me.

Hide

Permalink

Ted Yu added a comment - 08/Feb/13 22:23

Integrated to trunk.

Thanks for the patch, Sergey.

Thanks for the review, Jimmy and Matteo.

Show

Ted Yu added a comment - 08/Feb/13 22:23 Integrated to trunk. Thanks for the patch, Sergey. Thanks for the review, Jimmy and Matteo.

Hide

Permalink

Hadoop QA added a comment - 08/Feb/13 22:23

+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 .

This message is automatically generated.

Show

Hadoop QA added a comment - 08/Feb/13 22:23 +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.

Hide

Permalink

Hudson added a comment - 09/Feb/13 00:17

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

Show

Hudson added a comment - 09/Feb/13 00:17 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

Hide

Permalink

Hudson added a comment - 09/Feb/13 01:41

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

Show

Hudson added a comment - 09/Feb/13 01:41 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

Hide

Permalink

stack added a comment - 23/Sep/13 18:31

Marking closed.

Show

stack added a comment - 23/Sep/13 18:31 Marking closed.

refactor storefile management in HStore in order to support things like LevelDB-style compactions

Details

Attachments

Issue Links

Activity

People

Dates

Development

Agile