virajjasani commented on a change in pull request #1784:
URL: https://github.com/apache/hbase/pull/1784#discussion_r430899478
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1921,9 +1921,15 @@ public boolean shouldPerformMajorCompaction() throws
IOException {
// If we're enqueuing a major, clear the force flag.
this.forceMajor = this.forceMajor && !request.isMajor();
- // Set common request properties.
- // Set priority, either override value supplied by caller or from
store.
- request.setPriority((priority != Store.NO_PRIORITY) ? priority :
getCompactPriority());
+ if (request.isAfterSplit()) {
Review comment:
Agree, we should override priority only if it is higher than constant
(MIN_VALUE + 1000).
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionRequestImpl.java
##########
@@ -149,6 +158,7 @@ public int hashCode() {
result = prime * result + ((storeName == null) ? 0 : storeName.hashCode());
result = prime * result + (int) (totalSize ^ (totalSize >>> 32));
result = prime * result + ((tracker == null) ? 0 : tracker.hashCode());
+ result = prime * result + (isAfterSplit ? 1231 : 1237);
Review comment:
Yeah right, I exactly looked for an existing boolean and applied same
formula rather than regenerating hashcode() by IDE :)
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1921,9 +1921,15 @@ public boolean shouldPerformMajorCompaction() throws
IOException {
// If we're enqueuing a major, clear the force flag.
this.forceMajor = this.forceMajor && !request.isMajor();
- // Set common request properties.
- // Set priority, either override value supplied by caller or from
store.
- request.setPriority((priority != Store.NO_PRIORITY) ? priority :
getCompactPriority());
+ if (request.isAfterSplit()) {
+ // If the store belongs to recently splitted daughter regions,
better we consider
+ // them with the highest priority in the compaction queue.
+ request.setPriority(Integer.MIN_VALUE);
Review comment:
Yeah, MIN_VALUE+1000 (or 5000) should be able to provide more than
enough room for tasks that can emerge as even higher priority than split
housekeeping in future.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
