[ 
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

Reply via email to