-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-26583 Make StatisticThread alternative in BucketCache #3956
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
base: master
Are you sure you want to change the base?
Conversation
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
Have rebased to master. Let's see the UT result this time. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
| private transient final ScheduledExecutorService scheduleThreadPool = | ||
| Executors.newScheduledThreadPool(1, | ||
| new ThreadFactoryBuilder().setNameFormat("BucketCacheStatsExecutor").setDaemon(true).build()); | ||
| private transient ScheduledExecutorService scheduleThreadPool; |
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.
It could still be final?
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.
Fixed in the latest commit.
| "hbase.bucketcache.persistent.file.integrity.check.algorithm"; | ||
| private static final String DEFAULT_FILE_VERIFY_ALGORITHM = "MD5"; | ||
| private static final String STAT_THREAD_ENABLE_KEY = "hbase.bucketcache.stat.enable"; | ||
| private static final boolean STAT_THREAD_ENABLE_DEFAULT = false; |
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.
And better set it to true to keep the old behavior by default?
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.
The purpose to make the log alternative is that the log is only used when heavy debugging is needed. Turn on is a little bit annoying at other time I think. So that I changed the old behaviour. What do you think?
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 in this PR we'd better keep the old behavior, i.e, set it true. We can file another issue to change it to true on master branch, and mark it as incompatible change. On branch-2.x we'd better keep the old behavior.
|
🎊 +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. |
|
Please rebase and fix the checkstyle issue? Thanks. |
https://issues.apache.org/jira/browse/HBASE-26583