apurtell commented on a change in pull request #3652:
URL: https://github.com/apache/hbase/pull/3652#discussion_r808391314
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -218,14 +221,14 @@ public synchronized boolean requestSplit(final Region r) {
return false;
}
- public synchronized void requestSplit(final Region r, byte[] midKey) {
+ private synchronized void requestSplit(final Region r, byte[] midKey) {
Review comment:
What is the reason to change the visibility of this method?
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -218,14 +221,14 @@ public synchronized boolean requestSplit(final Region r) {
return false;
}
- public synchronized void requestSplit(final Region r, byte[] midKey) {
+ private synchronized void requestSplit(final Region r, byte[] midKey) {
requestSplit(r, midKey, null);
}
/*
* The User parameter allows the split thread to assume the correct user
identity
*/
- public synchronized void requestSplit(final Region r, byte[] midKey, User
user) {
+ private synchronized void requestSplit(final Region r, byte[] midKey, User
user) {
Review comment:
What is the reason to change the visibility of this method?
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
##########
@@ -253,6 +253,10 @@
public static final String COMPACTION_AFTER_BULKLOAD_ENABLE =
"hbase.compaction.after.bulkload.enable";
+ /** Config for allow split when file count greater than the configured
blocking file count*/
+ public static final String ENABLE_SPLIT_BEFORE_COMPACT_IGNORE_BLOCKING_KEY =
Review comment:
On the JIRA you (IMHO) convincingly argue this is a bug:
> But the region's compact priority is the minimum of all the stores, when
the number of storefiles in a store is larger than the configed
`hbase.hstore.blockingStoreFiles`, the priority will be a negative number, but
the compared priority in requestSplit() is 1(PRIORITY_USER).
So should we treat this like a bug fix? Blocking file limit should not
prevent splits. If so, there is no need for a new configuration setting here.
Now it can be argued that splitting a region that has so many store files is
suboptimal. That is true. It can be addressed by a follow up issue as an
optimization. (Not sure, perhaps split procedure can be modified to do
compaction inline in such degenerate cases, but anyway is an orthogonal
conversation.)
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -486,10 +489,10 @@ public int getSplitQueueSize() {
return splits.getQueue().size();
}
- private boolean shouldSplitRegion() {
- if(server.getNumberOfOnlineRegions() > 0.9*regionSplitLimit) {
+ private boolean allowSplitRegion() {
Review comment:
Seems like an unnecessary rename but it's a private method so not a big
deal.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
