[ https://issues.apache.org/jira/browse/HBASE-8884?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13701488#comment-13701488 ]
Chao Shi commented on HBASE-8884: --------------------------------- Thanks for reviewing this patch so quickly. {quote} Why is SimpleRpcScheduler embedded in RpcServer ? I would expect RpcScheduler and SimpleRpcScheduler to have their own classes. {quote} {quote} Yeah, move out the Scheduler implemetation into its own class (protected) in the rpc package {quote} It has to access some member of RpcServer (for now, getQosLevel and highPriorityLevel). It would be nice to have it a separate class when we have a approach to pass configuration to a scheduler. I'm trying to work on that. {quote} I'd think we would be done w/ header when running Calls? (Doesn't Connection object have what you need?) {quote} header is used by SimpleRpcScheduler to determine qos level. How to get it from connection? {quote} This is kinda ugly: MONITORED_RPC – it being static final in the class. Won't it be shared by master and regionserver when we run standalone w/ all threads in the one jvm? {quote} I agree. Yes, it is shared. It is only used for holder a thread-local MonitoredRpcHandler per handler thread. Any better ideas? {quote} Why we need a CallTask? Isn't a Call a Task? Would CallRunner be a better name? Or what is it doing? It is executing/running the call? {quote} CallRunner is OK for me. It is a Call plus the processing logic (a runnable), thus a scheduler can simply put it into a thread pool for execution. In addition, I would prefer adding a interface for Call (which exposes getters that a scheduler needs), so we can simply mock it up in testing. {quote} The difference between master and regionserver RpcServer are scheduler parameters – the pool sizes? Could you make a SchedulerConfiguration object that needs to be filled out and passed to RpcServer constructor? Or just have a masterschedulerconfiguration and a regionserverschedulerconfiguration that you pass to the RpcServer on construction? {quote} Good. Please note that besides pool size, a scheduler may have its own things configurable (implementation dependent), which may e between master and regionserver. In master, we can rename all "hbase.master.rpc.scheduler.\*" to "hbase.rpc.scheduler.\*", so the scheduler can read it. {quote} Maybe write up a one pager on what your plan is. It'd be easier to decipher your intent and be able to help instead of reading a patch. {quote} In fact, I haven't got idea what a fair scheduler exactly is. It is hard to have a algorithm fit all people/scenario. This patch is the very first step. With it, so we are able to test our implementation without modifying code inside RpcServer. I'm glad to discuss with you about the scheduler implementation in another jira issue (we're getting some testing data from the prototype scheduler). I don't have access to my devbox during weekend. I will submit a new patch in Mon or Tue. Thanks! > Pluggable RpcScheduler > ---------------------- > > Key: HBASE-8884 > URL: https://issues.apache.org/jira/browse/HBASE-8884 > Project: HBase > Issue Type: Improvement > Components: IPC/RPC > Reporter: Chao Shi > Attachments: hbase-8884.patch > > > Today, the RPC scheduling mechanism is pretty simple: it execute requests in > isolated thread-pools based on their priority. In the current implementation, > all normal get/put requests are using the same pool. We'd like to add some > per-user or per-region level isolation, so that a misbehaved user/region will > not saturate the thread-pool and cause DoS to others easily. The idea is > similar to FairScheduler in MR. The current scheduling code is not standalone > and is mixed with others (Connection#processRequest). The issue is the first > step to extract it to an interface, so that people are free to write and test > their own implementations. > This patch doesn't make it completely pluggable yet, as some parameters are > pass from constructor. This is because HMaster and HRegionServer both use > RpcServer and they have different thread-pool size config. Let me know if you > have a solution to this. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira