stack
added a comment - 08/May/12 06:32 I sort of follow. I don't understand the bit where you say "But here we read the old config item only!'. Help me out Anoop.
HBASE-3272 is what changed this config. Looking at the patch over there, will it help explaining why code is the way it is? (Maybe it doesn't!)

As per this change the default value for the hbase.hstore.blockingStoreFiles is been considered as 7. Previously it was like 1+ value for hbase.hstore.compactionThreshold[See the code below which checks if (this.blockingStoreFilesNumber == -1)]

I think now after the patch for HBASE-3272 this if check may not be needed

I didnt know about this patch based change and when seen this piece of code with if check for -1, it looks like if explicitely -1 is configured, it will take the blockingStoreFilesNumber as 1+ value of hbase.hstore.compactionThreshold. I thought that this was intentionally given.. What this defect tells is as we later changed the config name for hbase.hstore.compactionThreshold to hbase.hstore.compaction.min, we need to consider that value here with precedence over the value for hbase.hstore.compactionThreshold

Hope the desc is clear to you now... Any way now that is not needed.. What I think is that we can remove the code

Anoop Sam John
added a comment - 08/May/12 08:33 @Stack
I have gone through the patch for HBASE-3272
- conf.getInt( "hbase.hstore.blockingStoreFiles" , -1);
+ conf.getInt( "hbase.hstore.blockingStoreFiles" , 7);
if ( this .blockingStoreFilesNumber == -1) {
this .blockingStoreFilesNumber = 1 +
conf.getInt( "hbase.hstore.compactionThreshold" , 3);
As per this change the default value for the hbase.hstore.blockingStoreFiles is been considered as 7. Previously it was like 1+ value for hbase.hstore.compactionThreshold
[See the code below which checks if (this.blockingStoreFilesNumber == -1)]
I think now after the patch for HBASE-3272 this if check may not be needed
if ( this .blockingStoreFilesNumber == -1) {
this .blockingStoreFilesNumber = 1 +
conf.getInt( "hbase.hstore.compactionThreshold" , 3);
I didnt know about this patch based change and when seen this piece of code with if check for -1, it looks like if explicitely -1 is configured, it will take the blockingStoreFilesNumber as 1+ value of hbase.hstore.compactionThreshold. I thought that this was intentionally given.. What this defect tells is as we later changed the config name for hbase.hstore.compactionThreshold to hbase.hstore.compaction.min, we need to consider that value here with precedence over the value for hbase.hstore.compactionThreshold
Hope the desc is clear to you now... Any way now that is not needed.. What I think is that we can remove the code
if ( this .blockingStoreFilesNumber == -1) {
this .blockingStoreFilesNumber = 1 +
conf.getInt( "hbase.hstore.compactionThreshold" , 3);
which might confuse us.. What do you say Stack.

-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.

+1 hadoop23. The patch compiles against the hadoop 0.23.x 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.

Hadoop QA
added a comment - 08/May/12 19:38 -1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12526008/HBASE-5925.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.
+1 hadoop23. The patch compiles against the hadoop 0.23.x 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 core tests. The patch passed unit tests in .
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1799//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1799//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1799//console
This message is automatically generated.

Hudson
added a comment - 08/May/12 22:29 Integrated in HBase-TRUNK #2857 (See https://builds.apache.org/job/HBase-TRUNK/2857/ )
HBASE-5925 Issue with only using the old config param hbase.hstore.compactionThreshold but not the corresponding new one (Revision 1335738)
Result = FAILURE
stack :
Files :
/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreFlusher.java

Hudson
added a comment - 09/May/12 05:54 Integrated in HBase-TRUNK-security #196 (See https://builds.apache.org/job/HBase-TRUNK-security/196/ )
HBASE-5925 Issue with only using the old config param hbase.hstore.compactionThreshold but not the corresponding new one (Revision 1335738)
Result = FAILURE
stack :
Files :
/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreFlusher.java