[
https://issues.apache.org/jira/browse/HBASE-3290?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12965795#action_12965795
]
HBase Review Board commented on HBASE-3290:
-------------------------------------------
Message from: st...@duboce.net
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1263/#review2018
-----------------------------------------------------------
Ship it!
This looks great. I love the test. There are some comments below. See what
you think. I did not dig in deep on the algo but looks good.
trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
<http://review.cloudera.org/r/1263/#comment6361>
Good. I like the way you keep around old name.
FYI, there's white space on end of some of these lines of yours.
trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
<http://review.cloudera.org/r/1263/#comment6364>
Is this right? We check all storefiles for references where before we only
checked the subset of candidate compaction files for references?
(Hmm.. maybe the old stuff was wrong?)
trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
<http://review.cloudera.org/r/1263/#comment6362>
White space
trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
<http://review.cloudera.org/r/1263/#comment6365>
Good
trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
<http://review.cloudera.org/r/1263/#comment6366>
I don't grok this comment
trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
<http://review.cloudera.org/r/1263/#comment6367>
So, its ok to mess w/ file order? We won't get ourselves into trouble if
we don't respect the order in which files were written? We do a merge sort
when we read all compaction candidates in so should be fine I suppose -- since
its same as how scanner merges them......
Just asking because in old days order was important but I suppose we let go
of that a while back?
trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
<http://review.cloudera.org/r/1263/#comment6368>
Is this a good name for this method? We're compacting a Store, not Stores,
right?
trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
<http://review.cloudera.org/r/1263/#comment6369>
Nice
trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java
<http://review.cloudera.org/r/1263/#comment6370>
Excellent! I love you mocking up StoreFiles rather than fire up minicluster
FYI... loads of white space in here.
- stack
> Max Compaction Size
> -------------------
>
> Key: HBASE-3290
> URL: https://issues.apache.org/jira/browse/HBASE-3290
> Project: HBase
> Issue Type: Improvement
> Reporter: Nicolas Spiegelberg
> Assignee: Nicolas Spiegelberg
> Priority: Minor
>
> Add ability to specify a maximum storefile size for compaction. After this
> limit, we will not include this file in compactions. This is useful for
> large object stores and clusters that pre-split regions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
