-
Notifications
You must be signed in to change notification settings - Fork 66
Disable bucket cache on HBase masters #1277
Disable bucket cache on HBase masters #1277
Conversation
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.
Sure, looks good to me.
Notes for those deploying: This might restart regionservers, since they share an env file with masters. It's recommended to stop chef and update them in a loop. BETTER SAFE THAN SORRY. ™️ |
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 double check my read through if you would.
@@ -134,7 +134,8 @@ | |||
# If HBASE bucket cache is enabled the properties from this section will be included in hbase-site.xml | |||
# | |||
bucketcache_size = (node['bcpc']['hadoop']['hbase_rs']['mx_dir_mem']['size'] - node['bcpc']['hadoop']['hbase_rs']['hdfs_dir_mem']['size']).floor | |||
if node['bcpc']['hadoop']['hbase']['bucketcache']['enabled'] == true | |||
if node['bcpc']['hadoop']['hbase']['bucketcache']['enabled'] == true && | |||
node['bcpc']['hadoop']['rs_hosts'].map { |rs| rs.values[0] }.include?(node['fqdn']) |
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.
Using include
may cause foo-r1n1 to match foo-r1n10. Why not a select
over map
and look for equality?
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 don't think that will happen because the result of a map is an Array.
It will call the include? which falls back to the method in Enumerable.
That returns true if any element in the map results == node['fqdn']
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.
What Ron said. In the map, I strip the small hashes down to just the value. The result is a String
array of just FQDNs. I then do an include?
using the fqdn of the node itself to check for a match, which should avoid collisions.
LGTM. |
- Makes it more readable and simpler.
Addressed concerns in this review.
This removes the config parameters from HBase masters that are added when the bucket cache is enabled. These were only intended for region servers anyway.
EDIT: This was tested on a dev hardware cluster.
EDIT2: The second iteration was tested in the same place.