-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-27362 CompactSplit.requestCompactionInternal may bypass compact… #4768
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ionsEnabled check
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
| if (midKey == null) { | ||
| LOG.debug("Region " + r.getRegionInfo().getRegionNameAsString() | ||
| + " not splittable because midkey=null"); | ||
| if (LOG.isDebugEnabled()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these logging changes related?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, it is not related, just modified by the way.
| // re-create executor pool if compactions are disabled. | ||
| if (!isCompactionsEnabled()) { | ||
| LOG.info("Re-Initializing compactions because user switched on compactions"); | ||
| if (LOG.isInfoEnabled()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer we keep the old style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I have reverted it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to align them, I prefer we open a new issue.
| protected void requestCompactionInternal(HRegion region, HStore store, String why, int priority, | ||
| boolean selectNow, CompactionLifeCycleTracker tracker, | ||
| CompactionCompleteTracker completeTracker, User user) throws IOException { | ||
| if (!this.isCompactionsEnabled()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this is the actual fix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it is the actual fix.
|
🎊 +1 overall
This message was automatically generated. |
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
Show resolved
Hide resolved
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
…ionsEnabled check (#4768) Co-authored-by: comnetwork <[email protected]> Signed-off-by: Duo Zhang <[email protected]> (cherry picked from commit a26cbf1)
…ionsEnabled check (#4768) Co-authored-by: comnetwork <[email protected]> Signed-off-by: Duo Zhang <[email protected]> (cherry picked from commit a26cbf1)
…ionsEnabled check (apache#4768) Co-authored-by: comnetwork <[email protected]> Signed-off-by: Duo Zhang <[email protected]> (cherry picked from commit a26cbf1)
|
Nice one 👍 |
…ionsEnabled check (apache#4768) Co-authored-by: comnetwork <[email protected]> Signed-off-by: Duo Zhang <[email protected]> (cherry picked from commit a26cbf1) (cherry picked from commit b733485) Change-Id: Ie58f7748ff11422e1d76b405c1bf6c2989b8f649
…ionsEnabled check