-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-23968 Periodically check whether a system stop is requested in compaction by time. #1274
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
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
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.
This is a sweet patch. Nice little bit of cleanup. Left some minor comments below.
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CloseChecker.java
Show resolved
Hide resolved
* Check periodically to see if a system stop is requested | ||
*/ | ||
@InterfaceAudience.Private | ||
public class CloseChecker { |
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.
Does it have to be public? Can it be package local? It is only used in this package?
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 think it would be good to use package local
.
However, The package of FaultyMobStoreCompactor
is not the same as CloseChecker
.
The package of FaultyMobStoreCompactor
is org.apache.hadoop.hbase.mob
.
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.
dang.
ok. public for now.
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CloseChecker.java
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/DefaultMobStoreCompactor.java
Outdated
Show resolved
Hide resolved
this.lastCloseCheckMillis = currentTime; | ||
} | ||
|
||
public boolean isClosedBySizeLimit(Store store, long bytesWritten) { |
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.
s/isClosedBySizeLimit/isSizeLimit/g
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CloseChecker.java
Outdated
Show resolved
Hide resolved
@saintstack Thank you for your review. I reflected your feedback. |
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.
Approved pending QA build. Nice.
* Check periodically to see if a system stop is requested | ||
*/ | ||
@InterfaceAudience.Private | ||
public class CloseChecker { |
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.
dang.
ok. public for now.
retest build |
retest this please |
I started the qabot again |
@busbey Thank you!! |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@saintstack I fixed checkstyle error. But I don't know why unit was failed. |
Hard to tell. It failed making the report. See below. Let me try locally and if good I'll commit.
|
Let me retry the run here too. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@saintstack Thank you! |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@saintstack May I ask you to confirm it? |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
|
The failure of
|
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@saintstack All test is green. Could you check this PR? |
Sorry for taking so long to get back to this @mwkang I merged to master. I tried to backport to branch-2 but conflicts. Please put up a new PR and I'll merge it back. You might have to do one for branch-1 too. Thanks. |
Thanks you. Okay I will. |
The compaction check that the system is stopped when it reaches hbase.hstore.close.check.interval (size) or hbase.hstore.close.check.time.interval (time)