Github user JamesRTaylor commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/55#discussion_r27270740
  
    --- Diff: 
phoenix-core/src/main/java/org/apache/hadoop/hbase/ipc/PhoenixRpcScheduler.java 
---
    @@ -40,28 +40,34 @@
         private static final int DEFAULT_MAX_CALLQUEUE_LENGTH_PER_HANDLER = 10;
     
         private RpcScheduler delegate;
    -    private int minPriority;
    -    private int maxPriority;
    -    private RpcExecutor callExecutor;
    +    private int indexPriority;
    +    private int metadataPriority;
    +    private RpcExecutor indexCallExecutor;
    +    private RpcExecutor metadataCallExecutor;
         private int port;
     
    -    public PhoenixIndexRpcScheduler(int indexHandlerCount, Configuration 
conf,
    -            RpcScheduler delegate, int minPriority, int maxPriority) {
    +    public PhoenixRpcScheduler(int indexHandlerCount, int 
metadataHandlerCount, Configuration conf,
    --- End diff --
    
    I don't think declaring a range of priorities future proofs this as no one 
will know we've reserved that range - folks don't even read docs in general. If 
if they did happen upon it, there's nothing that'll enforce that they respect 
it. Better IMO to declare that we're using two explicit priorities. If folks 
need to change them, it pretty straightforward as it's two documented config 
parameters.
    
    The current implementation is simpler, removes a number of configs (we 
already have too many), and meets the needs.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to