-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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-28517 Make properties dynamically configured #5823
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Just make CacheConfig implements ConfigurationObserver and register it in HStore.registerChildren? And if we want to change these flags dynamically, I think they should be marked as volatile? Thanks. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
@Apache9 I ran the failed tests locally, they passed. Can you please review the new patch set? |
Hi @Apache9 Can you please review this one and let me know your comments? |
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.
Overall LGTM.
Just a minor comment issue.
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
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.
LGTM, minor comment on the log message.
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
🎊 +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. |
Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Peter Somogyi <psomogyi@apache.org> (cherry picked from commit 7122da5)
Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Peter Somogyi <psomogyi@apache.org> (cherry picked from commit 7122da5)
Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Peter Somogyi <psomogyi@apache.org> (cherry picked from commit 7122da5)
Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Peter Somogyi <psomogyi@apache.org> (cherry picked from commit 7122da5)
Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Peter Somogyi <psomogyi@apache.org> (cherry picked from commit 7122da5)
Make following properties dynamically configured,
hbase.rs.evictblocksonclose
hbase.rs.cacheblocksonwrite
hbase.block.data.cacheonread
for use case scenarios where configuring them dynamically would help in achieving better throughput.