HBase
  1. HBase
  2. HBASE-11048

Support setting custom priority per client RPC

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.99.0, 0.98.2
    • Fix Version/s: 0.99.0, 0.98.3
    • Component/s: Client
    • Labels:

      Description

      Servers have the ability to handle custom rpc priority levels, but currently we are only using it to differentiate META/ROOT updates from replication and other 'priority' updates (as specified by annotation tags per RS method).

      However, some clients need the ability to create custom handlers (e.g. PHOENIX-938) which can really only be cleanly tied together to requests by the request priority. The disconnect is in that there is no way for the client to overwrite the priority per table - the PayloadCarryingRpcController will always just set priority per ROOT/META and otherwise just use the generic priority.

      1. hbase-11048-trunk-v1.patch
        48 kB
        Jesse Yates
      2. hbase-11048-trunk-v0.patch
        32 kB
        Jesse Yates
      3. hbase-11048-0.98-v0.patch
        56 kB
        Jesse Yates

        Activity

        Hide
        Enis Soztutar added a comment -

        Closing this issue after 0.99.0 release.

        Show
        Enis Soztutar added a comment - Closing this issue after 0.99.0 release.
        Hide
        stack added a comment -

        +1 on patch

        Show
        stack added a comment - +1 on patch
        Hide
        Hudson added a comment -

        FAILURE: Integrated in HBase-0.98-on-Hadoop-1.1 #294 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/294/)
        HBASE-11048 Support setting custom priority per client RPC (jyates: rev 796134e9f5ebc97451d9e59bcc75fcb3d01b4a4d)

        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientSmallScanner.java
        • hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRpcControllerFactory.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/ReversedClientScanner.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncProcess.java
        • hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncProcess.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/ReversedScannerCallable.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientSmallReversedScanner.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcControllerFactory.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RegionCoprocessorRpcChannel.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/DelegatingPayloadCarryingRpcController.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/MultiServerCallable.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALEditsReplaySink.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java
        Show
        Hudson added a comment - FAILURE: Integrated in HBase-0.98-on-Hadoop-1.1 #294 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/294/ ) HBASE-11048 Support setting custom priority per client RPC (jyates: rev 796134e9f5ebc97451d9e59bcc75fcb3d01b4a4d) hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientSmallScanner.java hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRpcControllerFactory.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/ReversedClientScanner.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncProcess.java hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncProcess.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/ReversedScannerCallable.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientSmallReversedScanner.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcControllerFactory.java hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RegionCoprocessorRpcChannel.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/DelegatingPayloadCarryingRpcController.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/MultiServerCallable.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALEditsReplaySink.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in HBase-0.98 #312 (See https://builds.apache.org/job/HBase-0.98/312/)
        HBASE-11048 Support setting custom priority per client RPC (jyates: rev 796134e9f5ebc97451d9e59bcc75fcb3d01b4a4d)

        • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALEditsReplaySink.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RegionCoprocessorRpcChannel.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcControllerFactory.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/DelegatingPayloadCarryingRpcController.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncProcess.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientSmallScanner.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/ReversedClientScanner.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/ReversedScannerCallable.java
        • hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRpcControllerFactory.java
        • hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncProcess.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientSmallReversedScanner.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/MultiServerCallable.java
        Show
        Hudson added a comment - FAILURE: Integrated in HBase-0.98 #312 (See https://builds.apache.org/job/HBase-0.98/312/ ) HBASE-11048 Support setting custom priority per client RPC (jyates: rev 796134e9f5ebc97451d9e59bcc75fcb3d01b4a4d) hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALEditsReplaySink.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RegionCoprocessorRpcChannel.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcControllerFactory.java hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/DelegatingPayloadCarryingRpcController.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncProcess.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientSmallScanner.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/ReversedClientScanner.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/ReversedScannerCallable.java hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRpcControllerFactory.java hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncProcess.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientSmallReversedScanner.java hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/MultiServerCallable.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in HBase-TRUNK #5138 (See https://builds.apache.org/job/HBase-TRUNK/5138/)
        HBASE-11048 Support setting custom priority per client RPC (jyates: rev c61cb7fb55124547a36a6ef56afaec43676039f8)

        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientSmallScanner.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java
        • hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRpcControllerFactory.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncProcess.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/DelegatingPayloadCarryingRpcController.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/MultiServerCallable.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/ReversedClientScanner.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java
        • hbase-server/src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALEditsReplaySink.java
        • hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncProcess.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcControllerFactory.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientSmallReversedScanner.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionManager.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java
        • hbase-client/src/main/java/org/apache/hadoop/hbase/client/ReversedScannerCallable.java
        Show
        Hudson added a comment - FAILURE: Integrated in HBase-TRUNK #5138 (See https://builds.apache.org/job/HBase-TRUNK/5138/ ) HBASE-11048 Support setting custom priority per client RPC (jyates: rev c61cb7fb55124547a36a6ef56afaec43676039f8) hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientSmallScanner.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRpcControllerFactory.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncProcess.java hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/DelegatingPayloadCarryingRpcController.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/MultiServerCallable.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/ReversedClientScanner.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java hbase-server/src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALEditsReplaySink.java hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncProcess.java hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcControllerFactory.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientSmallReversedScanner.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionManager.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/ReversedScannerCallable.java
        Hide
        Andrew Purtell added a comment -

        No worries, the deadline is probably more like early afternoon tomorrow.

        Show
        Andrew Purtell added a comment - No worries, the deadline is probably more like early afternoon tomorrow.
        Hide
        Jesse Yates added a comment -

        Thanks for taking a look Andrew Purtell! I'll commit this evening (right under the RC deadline ), unless there are any objections

        Show
        Jesse Yates added a comment - Thanks for taking a look Andrew Purtell ! I'll commit this evening (right under the RC deadline ), unless there are any objections
        Hide
        Andrew Purtell added a comment -

        So this is all you need to achieve your objectives Jesse Yates I assume.

        I looked at the 0.98 patch. The changes are additions to existing interfaces or changes to classes with an internal audience. I will compare the next RC against earlier releases looking for variations in per op latencies. We can sink the RC and back out the change if there is a significant regression.

        Any objections to committing?

        Show
        Andrew Purtell added a comment - So this is all you need to achieve your objectives Jesse Yates I assume. I looked at the 0.98 patch. The changes are additions to existing interfaces or changes to classes with an internal audience. I will compare the next RC against earlier releases looking for variations in per op latencies. We can sink the RC and back out the change if there is a significant regression. Any objections to committing?
        Hide
        Jesse Yates added a comment -

        Attaching updated patch for trunk (rebased on current) and patch for 0.98.

        Ended up skipping the changes to CP to avoid touching the guts too much. We can do it later if someone has a use-case for it - right now just covering the internals of HTable.

        Show
        Jesse Yates added a comment - Attaching updated patch for trunk (rebased on current) and patch for 0.98. Ended up skipping the changes to CP to avoid touching the guts too much. We can do it later if someone has a use-case for it - right now just covering the internals of HTable.
        Hide
        Jesse Yates added a comment -

        Patch coming (hopefully) today.

        Show
        Jesse Yates added a comment - Patch coming (hopefully) today.
        Hide
        Andrew Purtell added a comment -

        Push to 0.98.4?

        Show
        Andrew Purtell added a comment - Push to 0.98.4?
        Hide
        Jesse Yates added a comment -

        Yeah, I got sidetracked by internal work and then it proved a bit more effort than in trunk. Specifically, the ProtobufUtil is used all over the place, rather than using the PayloadCarryingRpcController directly in HTable. Which means adding a lot of methods for backward compatibility and a lot places where it would be easy to miss using the controller.

        Open question: should we support a custom controller factory for:

        • coprocessors
        • admin functions
        • master calls
        • internal system table look ups (e.g. determining target region in HConnection)

        My gut says yes, no, no, no, respectively. Any thoughts?

        Show
        Jesse Yates added a comment - Yeah, I got sidetracked by internal work and then it proved a bit more effort than in trunk. Specifically, the ProtobufUtil is used all over the place, rather than using the PayloadCarryingRpcController directly in HTable. Which means adding a lot of methods for backward compatibility and a lot places where it would be easy to miss using the controller. Open question: should we support a custom controller factory for: coprocessors admin functions master calls internal system table look ups (e.g. determining target region in HConnection) My gut says yes, no, no, no, respectively. Any thoughts?
        Hide
        Andrew Purtell added a comment -

        This missed 0.98.2. Perhaps for 0.98.3 next month?

        Show
        Andrew Purtell added a comment - This missed 0.98.2. Perhaps for 0.98.3 next month?
        Hide
        Andrew Purtell added a comment -

        I'm not sure how Andrew Purtell feels about your work going into 0.98, so this issue might be the way forward for that until you finish up the work for trunk?

        Since you asked. Both sound like incremental improvements. We could take one, then the other. Caveat: Existing public interfaces aren't broken in 0.98, deprecation is fine. AsyncProcess and MultiServerCallable are package scoped so those are fine. ClientScanner is Public Stable, but it is just missing one backwards compatible constructor. Ditto ClientSmallScanner.

        Show
        Andrew Purtell added a comment - I'm not sure how Andrew Purtell feels about your work going into 0.98, so this issue might be the way forward for that until you finish up the work for trunk? Since you asked. Both sound like incremental improvements. We could take one, then the other. Caveat: Existing public interfaces aren't broken in 0.98, deprecation is fine. AsyncProcess and MultiServerCallable are package scoped so those are fine. ClientScanner is Public Stable, but it is just missing one backwards compatible constructor. Ditto ClientSmallScanner.
        Hide
        Jesse Yates added a comment -

        Matteo Bertozzi I think what I'm doing with making the payload rpc controller pluggable actually makes the following:

        because once you give the setPriority() to the client, all the client requests will be HI priority and that will impact the META requests.

        less of an issue as it allows the client to plug in their own implementation, which could be a one smarter about which table gets which priority (so meta, root keep their high priority and other tables can be ordered on priority).

        Clearly, this is well short of what you are proposing, but maybe its a good intermediate step? Maybe it might save you some small amount of work?

        For phoenix, we are creating an HTable on the server side to write between RS for the mutable secondary indexes. So the priority is somewhat surfaced to client (ala config values), but not actually changing the API.

        I'm not sure how Andrew Purtell feels about your work going into 0.98, so this issue might be the way forward for that until you finish up the work for trunk? Just trying to figure out how we can all eat our cake

        Show
        Jesse Yates added a comment - Matteo Bertozzi I think what I'm doing with making the payload rpc controller pluggable actually makes the following: because once you give the setPriority() to the client, all the client requests will be HI priority and that will impact the META requests. less of an issue as it allows the client to plug in their own implementation, which could be a one smarter about which table gets which priority (so meta, root keep their high priority and other tables can be ordered on priority). Clearly, this is well short of what you are proposing, but maybe its a good intermediate step? Maybe it might save you some small amount of work? For phoenix, we are creating an HTable on the server side to write between RS for the mutable secondary indexes. So the priority is somewhat surfaced to client (ala config values), but not actually changing the API. I'm not sure how Andrew Purtell feels about your work going into 0.98, so this issue might be the way forward for that until you finish up the work for trunk? Just trying to figure out how we can all eat our cake
        Hide
        James Taylor added a comment -

        Our use case in Phoenix is not to surface this to clients. Instead it provides a means of maintaining secondary indexes while preventing deadlocks. See PHOENIX-938 for more detail.

        Show
        James Taylor added a comment - Our use case in Phoenix is not to surface this to clients. Instead it provides a means of maintaining secondary indexes while preventing deadlocks. See PHOENIX-938 for more detail.
        Hide
        Matteo Bertozzi added a comment -

        Jesse Yates the description is on the document here HBASE-10994.
        The idea is to avoid the current N-queues with the "priority" as selector,
        because once you give the setPriority() to the client, all the client requests will be HI priority and that will impact the META requests.
        The new scheduler will be also part of the quotas, so each user will get its own slot and the priority that you set will be reflected inside the user slot, reordering the user requests.
        There will also be another scheduler/queue type where the client will set a deadline instead of a priority, so you can say I want my query to be executed in 2sec.

        Show
        Matteo Bertozzi added a comment - Jesse Yates the description is on the document here HBASE-10994 . The idea is to avoid the current N-queues with the "priority" as selector, because once you give the setPriority() to the client, all the client requests will be HI priority and that will impact the META requests. The new scheduler will be also part of the quotas, so each user will get its own slot and the priority that you set will be reflected inside the user slot, reordering the user requests. There will also be another scheduler/queue type where the client will set a deadline instead of a priority, so you can say I want my query to be executed in 2sec.
        Hide
        Jonathan Hsieh added a comment -
        Show
        Jonathan Hsieh added a comment - HBASE-10994
        Hide
        Jesse Yates added a comment -

        Jonathan Hsieh do you have a link to the jira? or maybe Matteo Bertozzi has a moment to take a look at this?

        Show
        Jesse Yates added a comment - Jonathan Hsieh do you have a link to the jira? or maybe Matteo Bertozzi has a moment to take a look at this?
        Hide
        Jonathan Hsieh added a comment -

        Matteo Bertozzi should take a look – he recently posted a design doc and some code for some basic rpc scheduling for QoS purposes.

        Show
        Jonathan Hsieh added a comment - Matteo Bertozzi should take a look – he recently posted a design doc and some code for some basic rpc scheduling for QoS purposes.
        Hide
        Jesse Yates added a comment -

        I was trying avoid changing the interfaces. With the above patch, you set it per call (rpc) and let the policy defined in your rpc controller determine the priority in the request header.

        Show
        Jesse Yates added a comment - I was trying avoid changing the interfaces. With the above patch, you set it per call (rpc) and let the policy defined in your rpc controller determine the priority in the request header.
        Hide
        Elliott Clark added a comment -

        Why put it per client ? Why not per call as is currently in the request header ?

        Show
        Elliott Clark added a comment - Why put it per client ? Why not per call as is currently in the request header ?
        Hide
        Jesse Yates added a comment -

        Attaching initial patch. I went with adding another factory in keeping with the same pattern we are using the RpcCaller creation.

        We could combine them into the same factory (or use a wrapper factory that delegates to the correct factory for the specific object needed), but im not sure that it really gains us much beyond not passing around another object.

        Show
        Jesse Yates added a comment - Attaching initial patch. I went with adding another factory in keeping with the same pattern we are using the RpcCaller creation. We could combine them into the same factory (or use a wrapper factory that delegates to the correct factory for the specific object needed), but im not sure that it really gains us much beyond not passing around another object.

          People

          • Assignee:
            Jesse Yates
            Reporter:
            Jesse Yates
          • Votes:
            0 Vote for this issue
            Watchers:
            14 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development