[ 
https://issues.apache.org/jira/browse/HBASE-3160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12926363#action_12926363
 ] 

HBase Review Board commented on HBASE-3160:
-------------------------------------------

Message from: "Nicolas" <[email protected]>


bq.  On 2010-10-29 09:48:53, Kannan Muthukkaruppan wrote:
bq.  > 
trunk/src/main/java/org/apache/hadoop/hbase/regionserver/PriorityCompactionQueue.java,
 line 138
bq.  > <http://review.cloudera.org/r/1103/diff/2/?file=16198#file16198line138>
bq.  >
bq.  >     the comment around "if it is already present in the queue" needs to 
be updated.

will update


bq.  On 2010-10-29 09:48:53, Kannan Muthukkaruppan wrote:
bq.  > 
trunk/src/main/java/org/apache/hadoop/hbase/regionserver/PriorityCompactionQueue.java,
 line 149
bq.  > <http://review.cloudera.org/r/1103/diff/2/?file=16198#file16198line149>
bq.  >
bq.  >     I think here and in this diff overall (as well as the log outputs) 
readability will be much better if we flipped the sign of the priority... i.e. 
higher values imply higher priority.
bq.  >     
bq.  >     
bq.  >

yeah, priority ordering on heaps are always controversial.  I think we should 
keep it as a minheap unless there is a reason to switch (no permanent state = 
no problem).  The nice thing about this ordering is that you know Pri <= 0 is 
bad, no matter what your blocking limit is.


bq.  On 2010-10-29 09:48:53, Kannan Muthukkaruppan wrote:
bq.  > trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java, 
line 190
bq.  > <http://review.cloudera.org/r/1103/diff/2/?file=16199#file16199line190>
bq.  >
bq.  >     -1 and not Integer.MAX_VALUE?
bq.  >     
bq.  >     If blocking store files is not specified (i.e. user can tolerate any 
number of files), then as per your intent, don't you want PRIORITY_USER to be 
higher-pri always?

agreed.  I just used the original value from MemStoreFlusher, but your version 
makes the most sense.  Should this be a seperate jira though?  We would need to 
intestigate any other areas of code that would need to be changed by altering 
this default...


bq.  On 2010-10-29 09:48:53, Kannan Muthukkaruppan wrote:
bq.  > trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java, 
line 1330
bq.  > <http://review.cloudera.org/r/1103/diff/2/?file=16199#file16199line1330>
bq.  >
bq.  >     I think we should state it here (or somewhere) explicitly that 
smaller values implies higher priority.

should we mention that in the CompactSplitThread?


- Nicolas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1103/#review1704
-----------------------------------------------------------





> Compactions: Use more intelligent priorities for PriorityCompactionQueue
> ------------------------------------------------------------------------
>
>                 Key: HBASE-3160
>                 URL: https://issues.apache.org/jira/browse/HBASE-3160
>             Project: HBase
>          Issue Type: Improvement
>    Affects Versions: 0.89.20100924, 0.90.0
>            Reporter: Nicolas Spiegelberg
>            Assignee: Nicolas Spiegelberg
>             Fix For: 0.90.0
>
>         Attachments: HBASE-3160.patch
>
>
> One of the problems with the current compaction queue is that we have a very 
> low granularity on the importance of the various compactions in the queue.  
> If a StoreFile count exceeds 15 files, only then do we bump via enum change.  
> We should instead look into more intelligent, granular priority metrics for 
> choosing the next compaction.  

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to