-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-27126 Support multi-threads cleaner for MOB files #5833
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
HBASE-27126 Support multi-threads cleaner for MOB files #5833
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/MobFileCleanerChore.java
Outdated
Show resolved
Hide resolved
7716f46 to
f269274
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Please use shaded version of guava classes? Thanks. |
| public class MobFileCleanerChore extends ScheduledChore { | ||
|
|
||
| private static final Logger LOG = LoggerFactory.getLogger(MobFileCleanerChore.class); | ||
| public static final int TIME_OUT_VALUE = 30; |
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.
Please make it configurable and 30 minutes is too large value.
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.
added a configuration for it. default it set at 5 mins
| @BeforeClass | ||
| public static void setUpBeforeClass() throws Exception { | ||
| TEST_UTIL.getConfiguration().setInt("hfile.format.version", 3); | ||
| TEST_UTIL.getConfiguration().setInt(MOB_CLEANER_BATCH_SIZE_UPPER_BOUND, 2); |
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.
Shall we add UT for MOB_CLEANER_THREAD_COUNT > 1 also?
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.
added test case for multi thread and single thread scenarios
f269274 to
b482a58
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
b482a58 to
8a7bd16
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Any updates here? @chandrasekhar-188k Thanks. |
8a7bd16 to
b27697b
Compare
I have fixed the review comments and rebased the PR |
|
@pankaj72981 |
This comment has been minimized.
This comment has been minimized.
review comments are fixed, pls check the changes |
This comment has been minimized.
This comment has been minimized.
dc05a56 to
3ae9e1f
Compare
3ae9e1f to
ed1a1dd
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| MobFileCleanupUtil.cleanupObsoleteMobFiles(master.getConfiguration(), htd.getTableName(), | ||
| admin); | ||
| future.get(cleanerFutureTimeout, TimeUnit.SECONDS); | ||
| } catch (InterruptedException | ExecutionException | TimeoutException e) { |
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 the current thread is interrupted, future.get will return immediately with an InterruptedException...
The InterruptedException here is for interrupting the current thread, not the task in the thread pool.
| * run. | ||
| */ | ||
|
|
||
| public static void testCleanerInternal() throws Exception { |
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.
Why make this method public?
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.
updated to private
13a864e to
a499cb4
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/MobFileCleanerChore.java
Outdated
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/MobFileCleanerChore.java
Outdated
Show resolved
Hide resolved
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Pankaj Kumar <pankajkumar@apache.org>
Signed-off-by: Peng Lu <lupeng@apache.org> (cherry picked from commit f800a13)
Signed-off-by: Peng Lu <lupeng@apache.org> (cherry picked from commit f800a13)
Signed-off-by: Peng Lu <lupeng@apache.org> (cherry picked from commit f800a13)
No description provided.