HBASE-27756 Make request guardrail configs in RSRpcServices live updatable#5141
HBASE-27756 Make request guardrail configs in RSRpcServices live updatable#5141bbeaudreault merged 1 commit intoapache:masterfrom
Conversation
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
Will volatility of these fields introduce significant contention between handler threads? Should we also have each handler thread cache a thread-local copy of the value and refresh it periodically? |
|
Thanks for looking Nick! Good question. I'm far from an expert on the topic of cpu cache lines and such, but I've been curious about volatile for a while and googled it a few times in the past (again just now :)). There are numerous articles and SO answers out there, and it seems like the consensus is that on modern architectures the volatile read is uncontended and should be approximately the same performance of a normal read (so on the order of ns). It doesn't introduce any additional cpu opcodes and makes use of hardware memory barriers, which are close to free on x86+. It becomes a bit trickier when the value is being updated often, because then you introduce L1 cache invalidations, etc. But that's not relevant here since onConfigurationChange should be rare. I realize this is vague, but also I've never seen any indication in profiling of overhead of any of our existing volatiles (there are a bunch). To that point, this PR follows the common pattern for how we've been dealing with onConfigurationChange. There already a bunch of other volatiles in RpcServer, RpcExecutor, etc all related to this. You bring up a good point that it might be nice to unify all of these with per-handler caching but it might require some tricky refactoring given the classes involved. I think if we wanted to do that it might make sense to do it in a separate jira and only if we can find evidence that it'd provide a performance speedup, since it'd definitely increase complexity. Not sure it's worth it here for just these 3 volatiles, when there are a bunch more in other handler/rpc related classes. What do you think? |
|
You've convinced me. I would prefer simpler code if the |
…table (apache#5141) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>
…table (apache#5141) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>
No description provided.