Details

    • Type: New Feature New Feature
    • Status: Patch Available
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 0.90.2, 0.90.3, 0.90.4, 0.92.0
    • Fix Version/s: None
    • Component/s: master, regionserver
    • Labels:
      None
    • Release Note:
      Hide
      Patch used for table priority alone,In this stand alone patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.
      Show
      Patch used for table priority alone,In this stand alone patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

      Description

      The HBase isolation and allocation tool is designed to help users manage cluster resource among different application and tables.
      When we have a large scale of HBase cluster with many applications running on it, there will be lots of problems. In Taobao there is a cluster for many departments to test their applications performance, these applications are based on HBase. With one cluster which has 12 servers, there will be only one application running exclusively on this server, and many other applications must wait until the previous test finished.
      After we add allocation manage function to the cluster, applications can share the cluster and run concurrently. Also if the Test Engineer wants to make sure there is no interference, he/she can move out other tables from this group.
      In groups we use table priority to allocate resource, when system is busy; we can make sure high-priority tables are not affected lower-priority tables
      Different groups can have different region server configurations, some groups optimized for reading can have large block cache size, and others optimized for writing can have large memstore size.
      Tables and region servers can be moved easily between groups; after changing the configuration, a group can be restarted alone instead of restarting the whole cluster.

      git entry : https://github.com/ICT-Ope/HBase_allocation .

      We hope our work is helpful.

      1. Performance_of_Table_priority.pdf
        486 kB
        Liu Jia
      2. HBase_isolation_and_allocation_user_guide.pdf
        1.77 MB
        Liu Jia
      3. Design_document_for_HBase_isolation_and_allocation.pdf
        140 kB
        Liu Jia
      4. System Structure.jpg
        61 kB
        Liu Jia
      5. Design_document_for_HBase_isolation_and_allocation_Revised.pdf
        558 kB
        Liu Jia
      6. TablePriority.patch
        37 kB
        Liu Jia
      7. TablePriority_v8_for_trunk.patch
        74 kB
        Liu Jia
      8. TablePriority_v8.patch
        74 kB
        Ted Yu
      9. TablePriority_v8.patch
        76 kB
        Liu Jia
      10. TablePrioriy_v9.patch
        76 kB
        Liu Jia
      11. TablePriority_v12.patch
        78 kB
        Liu Jia
      12. TablePriority_v12.patch
        78 kB
        Liu Jia
      13. TablePriority_v15_with_coprocessor.patch
        79 kB
        Liu Jia
      14. TablePriority_v16_with_coprocessor.patch
        82 kB
        Liu Jia
      15. Simple_YCSB_Tests_For_TablePriority_Trunk_and_0.90.4.pdf
        494 kB
        Liu Jia
      16. TablePriority_v17.patch
        116 kB
        Liu Jia
      17. TablePriority_v17.patch
        116 kB
        Liu Jia

        Issue Links

          Activity

          Hide
          stack added a comment -

          Thank you for pointing out this interesting contribution. Over on github I see a tar.gz and a big patch. I could try and parse the patch to figure what it adds – I see lots of jsp and a bunch of new classes in a new allocation package that look intriguing. Any chance of a more in-depth description of what this contribution does? (How this high and low level grouping works, what is done to hbase to ensure priorities are respected, etc.)

          Thank you.

          Show
          stack added a comment - Thank you for pointing out this interesting contribution. Over on github I see a tar.gz and a big patch. I could try and parse the patch to figure what it adds – I see lots of jsp and a bunch of new classes in a new allocation package that look intriguing. Any chance of a more in-depth description of what this contribution does? (How this high and low level grouping works, what is done to hbase to ensure priorities are respected, etc.) Thank you.
          Hide
          Liu Jia added a comment -

          some testes of table priority

          Show
          Liu Jia added a comment - some testes of table priority
          Hide
          Liu Jia added a comment -

          the user guide and install instruction

          Show
          Liu Jia added a comment - the user guide and install instruction
          Hide
          Liu Jia added a comment -

          the design document

          Show
          Liu Jia added a comment - the design document
          Hide
          Liu Jia added a comment -

          Hi stack,we submitted three documents which may not very detailed.
          The design document explains how certain functionality be achieved including table priority and region server groups.
          If there are problems, I will follow and answer in this jira issue,
          Thanks for your suggestions and we wish to get more advice and help from you.

          Show
          Liu Jia added a comment - Hi stack,we submitted three documents which may not very detailed. The design document explains how certain functionality be achieved including table priority and region server groups. If there are problems, I will follow and answer in this jira issue, Thanks for your suggestions and we wish to get more advice and help from you.
          Hide
          stack added a comment -

          @Liu Thank you for posting the documents. That helps a lot. I'm impressed. You should post a note to the users's list describing what you have done. Others may be interested in using it.

          What would you like to see added to hbase to make your life easier writing this version of hbase? If coprocessors had been available when you went to write your customizations, could you have done them all up in coprocessors (It doesn't look like it given you have your own AssignmentManager and your own HBaseServer).

          What from your version of hbase could we put back into hbase core?

          Thank you for letting us know about this interesting application.

          Show
          stack added a comment - @Liu Thank you for posting the documents. That helps a lot. I'm impressed. You should post a note to the users's list describing what you have done. Others may be interested in using it. What would you like to see added to hbase to make your life easier writing this version of hbase? If coprocessors had been available when you went to write your customizations, could you have done them all up in coprocessors (It doesn't look like it given you have your own AssignmentManager and your own HBaseServer). What from your version of hbase could we put back into hbase core? Thank you for letting us know about this interesting application.
          Hide
          Andrew Purtell added a comment -

          It would be interesting what can be added to the Master coprocessor API to support this kind of extension.

          Show
          Andrew Purtell added a comment - It would be interesting what can be added to the Master coprocessor API to support this kind of extension.
          Hide
          Liu Jia added a comment -

          the relationship between groups and table priority

          Show
          Liu Jia added a comment - the relationship between groups and table priority
          Hide
          Liu Jia added a comment -

          We think the isolation and allocation tool is very compact,
          there are only two lines code used to change the "AssaingmentManager" and "HBaseServer" in HMaster and HRegionServer.
          If coprocessor can provide the entries which we can use to manipulate this two class, our development would be very convenient.
          Because our implementation has to change some private sub classes of HBaseServer,we have to rewrite them to suppress the original ones.

          In this version there are two functions which have little relationsh in between.
          The function of group involved with a JSP management portal,the table priority function is more light weight,
          users can set priority with shell or HBase api.
          We don't know whether one or both are convenient to add to core.

          Show
          Liu Jia added a comment - We think the isolation and allocation tool is very compact, there are only two lines code used to change the "AssaingmentManager" and "HBaseServer" in HMaster and HRegionServer. If coprocessor can provide the entries which we can use to manipulate this two class, our development would be very convenient. Because our implementation has to change some private sub classes of HBaseServer,we have to rewrite them to suppress the original ones. In this version there are two functions which have little relationsh in between. The function of group involved with a JSP management portal,the table priority function is more light weight, users can set priority with shell or HBase api. We don't know whether one or both are convenient to add to core.
          Hide
          Jeff Whiting added a comment -

          I would really like to see the priorities in hbase trunk. It seems really useful to prioritize tables so customer facing tables get high priority while batch processing tables get lower priority.

          I can see the usefulness of the isolation. However I have reservations because it seems to complicate hbase significantly and I'm unsure if others need the feature. Most of the time it seems people can just use different tables rather than do complete isolation.

          Show
          Jeff Whiting added a comment - I would really like to see the priorities in hbase trunk. It seems really useful to prioritize tables so customer facing tables get high priority while batch processing tables get lower priority. I can see the usefulness of the isolation. However I have reservations because it seems to complicate hbase significantly and I'm unsure if others need the feature. Most of the time it seems people can just use different tables rather than do complete isolation.
          Hide
          stack added a comment -

          @Jeff We have some notion already of 'priority' where .META. tables are favored over user-space ops. Could be expanded to do as you suggest. I don't think we'll be committing this work to TRUNK any time soon; it has alternate implementations of core classes – we'd need to figure a cleaner way of exposing the facility this customization needs to do its work. Meantime, this is interesting works and I could see folks who want this facility trying it out as is.

          Show
          stack added a comment - @Jeff We have some notion already of 'priority' where .META. tables are favored over user-space ops. Could be expanded to do as you suggest. I don't think we'll be committing this work to TRUNK any time soon; it has alternate implementations of core classes – we'd need to figure a cleaner way of exposing the facility this customization needs to do its work. Meantime, this is interesting works and I could see folks who want this facility trying it out as is.
          Hide
          Todd Lipcon added a comment -

          FWIW, I've heard rumblings out of Google that they do a similar thing to "group" the tablet servers for BigTable to isolate different applications in the same BT cell while keeping management overhead lower and allowing dynamic reprovisioning easily.

          Show
          Todd Lipcon added a comment - FWIW, I've heard rumblings out of Google that they do a similar thing to "group" the tablet servers for BigTable to isolate different applications in the same BT cell while keeping management overhead lower and allowing dynamic reprovisioning easily.
          Hide
          Liu Jia added a comment -

          @Jeff Thanks Jeff, I think to optimize the customer facing tables and control the resource of batch processing tables it's a good use case for priority.
          In Taobao Company, actually the group is used more often than table priority. Maybe without test or competition on resource, the table priority works quiet. But the group has a user portal and friendly with the users who not very familiar with HBase.
          The department which holds a shared HBase cluster called Taobao data platform, and many other departments may not very familiar with HBase want to use HBase. Most of them would like to test the performance (mostly is the throughput per region server) with their data and methods first. So the isolation part is very useful for them,
          I think if HBase wants to be a basic component of data center like Hadoop, a convenient and flexible way to isolate different projects is important.
          Jeff, is the mentioned part of the complicate of HBase significantly related with the Portal and too many JSP pages? What if add a shell tool to replace them?
          Because the actually implementation of group is just depends on region assignment and movement.

          Show
          Liu Jia added a comment - @Jeff Thanks Jeff, I think to optimize the customer facing tables and control the resource of batch processing tables it's a good use case for priority. In Taobao Company, actually the group is used more often than table priority. Maybe without test or competition on resource, the table priority works quiet. But the group has a user portal and friendly with the users who not very familiar with HBase. The department which holds a shared HBase cluster called Taobao data platform, and many other departments may not very familiar with HBase want to use HBase. Most of them would like to test the performance (mostly is the throughput per region server) with their data and methods first. So the isolation part is very useful for them, I think if HBase wants to be a basic component of data center like Hadoop, a convenient and flexible way to isolate different projects is important. Jeff, is the mentioned part of the complicate of HBase significantly related with the Portal and too many JSP pages? What if add a shell tool to replace them? Because the actually implementation of group is just depends on region assignment and movement.
          Hide
          Liu Jia added a comment -

          @stack Hi stack, when design the table priority part, we thought a good design may need to meet the following four requirements
          1, be efficient, don't affect the system throughput.
          2, when system is not busy, tables with different priorities no matter high or low can take full use of the system resources.
          3, when system is busy, high priority table costs more resource than low.
          4, when system is busy, low priority table will be functioning normal.
          Before carrying out this work, we actually thought about managing the block cache or memstore to achieve the goals, but this is more complex than managing the RPC queue and meanwhile it’s not a directly approach to solve this problem. Keep it simple is the principal that we must follow. Due to the LRU cache replacement strategy of block cache, the more RPC requests processed the more block cache size will be occupied, so we make this implementation decision.

          Stack, would you please send me the issue number which is about the previous discussion of this function. Other's opinions can help us find defective points and neglected problem in our design.

          Show
          Liu Jia added a comment - @stack Hi stack, when design the table priority part, we thought a good design may need to meet the following four requirements 1, be efficient, don't affect the system throughput. 2, when system is not busy, tables with different priorities no matter high or low can take full use of the system resources. 3, when system is busy, high priority table costs more resource than low. 4, when system is busy, low priority table will be functioning normal. Before carrying out this work, we actually thought about managing the block cache or memstore to achieve the goals, but this is more complex than managing the RPC queue and meanwhile it’s not a directly approach to solve this problem. Keep it simple is the principal that we must follow. Due to the LRU cache replacement strategy of block cache, the more RPC requests processed the more block cache size will be occupied, so we make this implementation decision. Stack, would you please send me the issue number which is about the previous discussion of this function. Other's opinions can help us find defective points and neglected problem in our design.
          Hide
          Liu Jia added a comment -

          @Todd Hi Todd, I think to provide HBase as a service may face the same problem of isolation and prioritization, this part is mentioned by Jeffrey Dean in one of his ppt. but just come up as the following "BigTable as a service ?
          – Interesting issues of resource fairness, performance isolation, prioritization, etc. across different clients"
          But without any further discussion, any more detailed information? The Same part in BigTable will be very interesting and valuable for to refine our designing.

          May be our group function could integrate with the user management part of HBase in the future.

          Show
          Liu Jia added a comment - @Todd Hi Todd, I think to provide HBase as a service may face the same problem of isolation and prioritization, this part is mentioned by Jeffrey Dean in one of his ppt. but just come up as the following "BigTable as a service ? – Interesting issues of resource fairness, performance isolation, prioritization, etc. across different clients" But without any further discussion, any more detailed information? The Same part in BigTable will be very interesting and valuable for to refine our designing. May be our group function could integrate with the user management part of HBase in the future.
          Hide
          Liu Jia added a comment -

          patch used for table priority alone,In this stand alone patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          Show
          Liu Jia added a comment - patch used for table priority alone,In this stand alone patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.
          Hide
          Liu Jia added a comment -

          Patch used for table priority alone,In this stand alone patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          Show
          Liu Jia added a comment - Patch used for table priority alone,In this stand alone patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.
          Hide
          Ted Yu added a comment -

          Thanks for the update.
          Please update patch for TRUNK - new feature doesn't go to 0.90 branch.

          +  public static class ActionPriPlus {
          

          How about naming the above class ActionPriorites ?

          +    public ActionPriPlus(int scanPlus, int putPlus, int getPlus, int deletePlus) {
          

          Get is implemented as Scan. Do we need to give them different priorities ?

          For PriorityJobQueue:

          +  private void addSize() {
          +    this.addLock.lock();
          +    this.size++;
          +    this.addLock.unlock();
          

          Why don't we check this.size against this.capacity to make it symmetrical with the method setSize() ? Should add javadoc.

          +  public PriorityJobQueue(int size, int lowestPrid, PriorityHBaseServer server) {
          

          size paramater should be renamed capacity.

          I see e.printStackTrace() and System.out.println(out) in several places. They should be replaced with LOG.error() and LOG.debug().

          In the future, please use https://reviews.apache.org/ for reviewing new feature so that context is clearer.

          Please add unit test(s) to
          1. show how the new classes are used
          2. verify their functionality.

          Good job.

          Show
          Ted Yu added a comment - Thanks for the update. Please update patch for TRUNK - new feature doesn't go to 0.90 branch. + public static class ActionPriPlus { How about naming the above class ActionPriorites ? + public ActionPriPlus( int scanPlus, int putPlus, int getPlus, int deletePlus) { Get is implemented as Scan. Do we need to give them different priorities ? For PriorityJobQueue: + private void addSize() { + this .addLock.lock(); + this .size++; + this .addLock.unlock(); Why don't we check this.size against this.capacity to make it symmetrical with the method setSize() ? Should add javadoc. + public PriorityJobQueue( int size, int lowestPrid, PriorityHBaseServer server) { size paramater should be renamed capacity. I see e.printStackTrace() and System.out.println(out) in several places. They should be replaced with LOG.error() and LOG.debug(). In the future, please use https://reviews.apache.org/ for reviewing new feature so that context is clearer. Please add unit test(s) to 1. show how the new classes are used 2. verify their functionality. Good job.
          Hide
          Ted Yu added a comment -

          Either of the following two should be provided:
          1. enhanced Web UI showing table priorities. It would be desirable to allow users to sort display by priority
          2. enhanced hbase shell command for getting/setting table priority

          +      for (HRegionInfo region : list) {
          +        int pri = DEFAULT_PRI;
          +        HTableDescriptor des = region.getTableDesc();
          

          The above call would be extremely costly (see HBASE-451)

          Thanks

          Show
          Ted Yu added a comment - Either of the following two should be provided: 1. enhanced Web UI showing table priorities. It would be desirable to allow users to sort display by priority 2. enhanced hbase shell command for getting/setting table priority + for (HRegionInfo region : list) { + int pri = DEFAULT_PRI; + HTableDescriptor des = region.getTableDesc(); The above call would be extremely costly (see HBASE-451 ) Thanks
          Hide
          Ted Yu added a comment -

          HRegion.getTableDesc() can be used in TRUNK.

          Show
          Ted Yu added a comment - HRegion.getTableDesc() can be used in TRUNK.
          Hide
          Ted Yu added a comment -
          +        || invo.getMethodName().endsWith("put")
          

          I think checkAndPut isn't covered by the above expression.
          Similarly, checkAndDelete should be covered as well.

          incrementColumnValue, increment should be treated the same way as put.

          Show
          Ted Yu added a comment - + || invo.getMethodName().endsWith( "put" ) I think checkAndPut isn't covered by the above expression. Similarly, checkAndDelete should be covered as well. incrementColumnValue, increment should be treated the same way as put.
          Hide
          Todd Lipcon added a comment -

          hmm, can't this be integrated with our existing "setQosFunction" capabilities? Am I missing something?

          Show
          Todd Lipcon added a comment - hmm, can't this be integrated with our existing "setQosFunction" capabilities? Am I missing something?
          Hide
          Ted Yu added a comment -

          setQosFunction uses priority from annotation to methods:

            @QosPriority(priority=HIGH_QOS)
          

          This feature provides dynamic prioritization for the methods.

          Show
          Ted Yu added a comment - setQosFunction uses priority from annotation to methods: @QosPriority(priority=HIGH_QOS) This feature provides dynamic prioritization for the methods.
          Hide
          Todd Lipcon added a comment -

          setQosFunction uses priority from annotation to methods:

          no, the current implementation (HRegionServer.QosFunction), uses the annotations. But, HBaseServer itself supports any QosFuncion, which can decide priority however it wants. In addition to the annotations, we also prioritize any accesses to META as QoS. So, the table-based priority is just a generalization of what we already do for META in this place.

          Show
          Todd Lipcon added a comment - setQosFunction uses priority from annotation to methods: no, the current implementation ( HRegionServer.QosFunction ), uses the annotations. But, HBaseServer itself supports any QosFuncion, which can decide priority however it wants. In addition to the annotations, we also prioritize any accesses to META as QoS. So, the table-based priority is just a generalization of what we already do for META in this place.
          Hide
          Ted Yu added a comment -
          +  private void testAdd(Job<T> j) {
          

          I think the above method should be called tryAdd.

          Show
          Ted Yu added a comment - + private void testAdd(Job<T> j) { I think the above method should be called tryAdd.
          Hide
          Liu Jia added a comment -

          I think use the annotation to methods could set method level priority, but this kind of priority is not consider table, may be user wants one talbe's "scan" faster than "put" and another table's "put" faster than "scan", in this patch RPC call's priority is caculated by its table priority plus this table's method plus priority and different tables can have different method plus priorities and different table priorities.
          Perhaps there is some problems and omissions,
          can annotation achieve different method priorities per tables?

          @QosPriority(priority=HIGH_QOS)
          Show
          Liu Jia added a comment - I think use the annotation to methods could set method level priority, but this kind of priority is not consider table, may be user wants one talbe's "scan" faster than "put" and another table's "put" faster than "scan", in this patch RPC call's priority is caculated by its table priority plus this table's method plus priority and different tables can have different method plus priorities and different table priorities. Perhaps there is some problems and omissions, can annotation achieve different method priorities per tables? @QosPriority(priority=HIGH_QOS)
          Hide
          Ted Yu added a comment -

          By 'method plus priority' I guess you mean method priority.

          Show
          Ted Yu added a comment - By 'method plus priority' I guess you mean method priority.
          Hide
          Liu Jia added a comment -
          {Either of the following two should be provided: 1. enhanced Web UI showing table priorities. It would be desirable to allow users to sort display by priority 2. enhanced hbase shell command for getting/setting table priority}

          What about a JSP Page to show and make use coult set priorities? There is a page in group part which used to set table priorities, so we just need to get this page out.

          {Get is implemented as Scan. Do we need to give them different priorities ?}

          Users may need different performance on get and scan, maybe leave it there makes no harm
          we will post our patch on review board soon..

          Show
          Liu Jia added a comment - {Either of the following two should be provided: 1. enhanced Web UI showing table priorities. It would be desirable to allow users to sort display by priority 2. enhanced hbase shell command for getting/setting table priority} What about a JSP Page to show and make use coult set priorities? There is a page in group part which used to set table priorities, so we just need to get this page out. {Get is implemented as Scan. Do we need to give them different priorities ?} Users may need different performance on get and scan, maybe leave it there makes no harm we will post our patch on review board soon..
          Hide
          Ted Yu added a comment -

          You can reuse that page. However, since this feature goes to TRUNK, you need to rewrite it with jamon.
          Refer to src/main/jamon/org/apache/hbase/tmpl.

          Displaying table priority on master UI is desirable.

          Show
          Ted Yu added a comment - You can reuse that page. However, since this feature goes to TRUNK, you need to rewrite it with jamon. Refer to src/main/jamon/org/apache/hbase/tmpl. Displaying table priority on master UI is desirable.
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          Review request for hbase.

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1155226
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1155226
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- Review request for hbase. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1155226 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1155226 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/#review1345
          -----------------------------------------------------------

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java
          <https://reviews.apache.org/r/1421/#comment3036>

          Wrap long line please.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java
          <https://reviews.apache.org/r/1421/#comment3034>

          This should be in the same case as "put".

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java
          <https://reviews.apache.org/r/1421/#comment3035>

          The variable should be named actionPriorities.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment3037>

          Year should be 2011.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment3038>

          White space.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment3033>

          Why not compare size against this.capacity ?

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment3032>

          InterruptedException shouldn't be ignored.
          You can wrap it in InterruptedIOException and rethrow.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment3039>

          Explanation for parameter should be on the same line.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment3041>

          PriorityAddTimes should start with lowercase p.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment3042>

          Please name this method increasePriority.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment3043>

          Please name this method increasePriority.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment3040>

          This is not needed.
          add() can be declared to throw InterruptedException.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment3044>

          Return value should be boolean.

          • Ted

          On 2011-08-09 13:38:32, Jia Liu wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/1421/

          -----------------------------------------------------------

          (Updated 2011-08-09 13:38:32)

          Review request for hbase.

          Summary

          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.

          https://issues.apache.org/jira/browse/HBase-4120

          Diffs

          -----

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1155226

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1155226

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing

          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch

          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/#review1345 ----------------------------------------------------------- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java < https://reviews.apache.org/r/1421/#comment3036 > Wrap long line please. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java < https://reviews.apache.org/r/1421/#comment3034 > This should be in the same case as "put". http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java < https://reviews.apache.org/r/1421/#comment3035 > The variable should be named actionPriorities. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment3037 > Year should be 2011. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment3038 > White space. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment3033 > Why not compare size against this.capacity ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment3032 > InterruptedException shouldn't be ignored. You can wrap it in InterruptedIOException and rethrow. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment3039 > Explanation for parameter should be on the same line. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment3041 > PriorityAddTimes should start with lowercase p. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment3042 > Please name this method increasePriority. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment3043 > Please name this method increasePriority. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment3040 > This is not needed. add() can be declared to throw InterruptedException. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment3044 > Return value should be boolean. Ted On 2011-08-09 13:38:32, Jia Liu wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-08-09 13:38:32) Review request for hbase. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1155226 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1155226 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-08-09 14:43:34.859289)

          Review request for hbase.

          Summary (updated)
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1155226
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1155226
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-08-09 14:43:34.859289) Review request for hbase. Summary (updated) ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1155226 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1155226 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          Todd Lipcon added a comment -

          I'm still not convinced that this can't be done using the existing QoSFunction support within HBaseRPC. Look at our existing implementation - it already does prioritize both by type of operation and by target table. It needs to be extended to use the table descriptors, and the queues in RpcServer need to be modified a bit more to be a priority queue, but the basics are already there. Subclassing is not the right approach.

          Show
          Todd Lipcon added a comment - I'm still not convinced that this can't be done using the existing QoSFunction support within HBaseRPC. Look at our existing implementation - it already does prioritize both by type of operation and by target table. It needs to be extended to use the table descriptors, and the queues in RpcServer need to be modified a bit more to be a priority queue, but the basics are already there. Subclassing is not the right approach.
          Hide
          stack added a comment -

          I'd also encourage study of existing QoSFunction. There would need to be a good reason for by-passing the existing prioritization chassis.

          Show
          stack added a comment - I'd also encourage study of existing QoSFunction. There would need to be a good reason for by-passing the existing prioritization chassis.
          Hide
          Ted Yu added a comment -

          Todd was referring to this queue in HBaseServer:

            protected BlockingQueue<Call> priorityCallQueue;
          

          See processData() method:

                Writable param = ReflectionUtils.newInstance(paramClass, conf);           // read param
                param.readFields(dis);
          
                Call call = new Call(id, param, this, responder);
          
                if (priorityCallQueue != null && getQosLevel(param) > highPriorityLevel) {
                  priorityCallQueue.put(call);
          

          where paramClass is currently ipc.Invocation

          Show
          Ted Yu added a comment - Todd was referring to this queue in HBaseServer: protected BlockingQueue<Call> priorityCallQueue; See processData() method: Writable param = ReflectionUtils.newInstance(paramClass, conf); // read param param.readFields(dis); Call call = new Call(id, param, this , responder); if (priorityCallQueue != null && getQosLevel(param) > highPriorityLevel) { priorityCallQueue.put(call); where paramClass is currently ipc.Invocation
          Hide
          Todd Lipcon added a comment -

          right – we would probably want to change the queue implementation over to something more advanced like Jia Liu has done, but we already have the code there that calls out to a prioriizing function which returns an int. So, I don't think we need to subclass HBaseServer to add that.

          Show
          Todd Lipcon added a comment - right – we would probably want to change the queue implementation over to something more advanced like Jia Liu has done, but we already have the code there that calls out to a prioriizing function which returns an int. So, I don't think we need to subclass HBaseServer to add that.
          Hide
          Ted Yu added a comment -

          See also QoSFunction.apply() in HRegionServer:

                // scanner methods...
                if (methodName.equals("next") || methodName.equals("close")) {
          ...
                    if (regionName.isMetaRegion()) {
                      // LOG.debug("High priority scanner request: " + scannerId);
                      return HIGH_QOS;
                    }
          

          Todd suggested checking against the table descriptor to determine priority for the method.

          Show
          Ted Yu added a comment - See also QoSFunction.apply() in HRegionServer: // scanner methods... if (methodName.equals( "next" ) || methodName.equals( "close" )) { ... if (regionName.isMetaRegion()) { // LOG.debug( "High priority scanner request: " + scannerId); return HIGH_QOS; } Todd suggested checking against the table descriptor to determine priority for the method.
          Hide
          Todd Lipcon added a comment -

          Ted asked me to add more detail to my comments here - sorry for being terse above. I was booted in Windows where I don't have access to the source code.

          In the patch on Review Board right now, PriorityHBaseServer inherits from Server to do the following:

          • sub out the callQueue object for a different implementation, using reflection no less. That's totally unacceptable style in my opinion – there has to be a better way to do it.
          • uses reflection to get access to certain private methods of HBaseRegionServer - again unacceptable
          • the queue that's interjected above calls getCallPriority on each Call object in order to determine where it belongs in the queue

          Instead, what I'm suggesting is:

          • extend the QosFunction that we already have implemented inside HRegionServer.java with the new logic, and move it out to its own class, since it's much more complicated with these additions. (perhaps retain the old "simple" one as a default implementation, and construct the QosFunction based on a config like hbase.regionserver.rpc.prioritizer or something)
          • modify the existing HBaseServer implementation so that, instead of just having two queues ("high" and "low" priority) it uses a priority queue – perhaps something like PriorityQueue<PrioritizedCall>, where PrioritizedCall is a wrapper around the int (returned from the qosFunction) and the original Call object, with compareTo set to compare priorities.
          • keep existing behavior of having multiple pools of handlers, where some handlers are reserved for high priority calls - this could either be generalized or left as is.

          I haven't looked much at the specific code, but I think the overall structure needs to be moved around a bit before we can get into the specific code review.

          Show
          Todd Lipcon added a comment - Ted asked me to add more detail to my comments here - sorry for being terse above. I was booted in Windows where I don't have access to the source code. In the patch on Review Board right now, PriorityHBaseServer inherits from Server to do the following: sub out the callQueue object for a different implementation, using reflection no less. That's totally unacceptable style in my opinion – there has to be a better way to do it. uses reflection to get access to certain private methods of HBaseRegionServer - again unacceptable the queue that's interjected above calls getCallPriority on each Call object in order to determine where it belongs in the queue Instead, what I'm suggesting is: extend the QosFunction that we already have implemented inside HRegionServer.java with the new logic, and move it out to its own class, since it's much more complicated with these additions. (perhaps retain the old "simple" one as a default implementation, and construct the QosFunction based on a config like hbase.regionserver.rpc.prioritizer or something) modify the existing HBaseServer implementation so that, instead of just having two queues ("high" and "low" priority) it uses a priority queue – perhaps something like PriorityQueue<PrioritizedCall>, where PrioritizedCall is a wrapper around the int (returned from the qosFunction) and the original Call object, with compareTo set to compare priorities. keep existing behavior of having multiple pools of handlers, where some handlers are reserved for high priority calls - this could either be generalized or left as is. I haven't looked much at the specific code, but I think the overall structure needs to be moved around a bit before we can get into the specific code review.
          Hide
          stack added a comment -

          Moving out of 0.90.3

          Show
          stack added a comment - Moving out of 0.90.3
          Hide
          Jean-Daniel Cryans added a comment -

          Bumping to 0.94, for sure it shouldn't be in 0.90 and I'd argue that 0.92 is choke full of new features.

          Show
          Jean-Daniel Cryans added a comment - Bumping to 0.94, for sure it shouldn't be in 0.90 and I'd argue that 0.92 is choke full of new features.
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-10-26 12:24:20.090230)

          Review request for hbase.

          Changes
          -------

          Move most of the priority function to PriorityFunction class. Make the PriorityHBaseServer much more slim.Fix all review comments.

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1155226
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1155226
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1155226
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-10-26 12:24:20.090230) Review request for hbase. Changes ------- Move most of the priority function to PriorityFunction class. Make the PriorityHBaseServer much more slim.Fix all review comments. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1155226 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1155226 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1155226 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/#review2867
          -----------------------------------------------------------

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6408>

          No year please.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6409>

          Remove extra 'the'

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6410>

          Should read 'the priority map, cache the region, table and'

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java
          <https://reviews.apache.org/r/1421/#comment6399>

          Year should be removed.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java
          <https://reviews.apache.org/r/1421/#comment6400>

          PriorityHBaseServer is not abstract, right ?

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java
          <https://reviews.apache.org/r/1421/#comment6402>

          Unless the line gets too wide, explanation would be on the same line as name of parameter.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java
          <https://reviews.apache.org/r/1421/#comment6403>

          What does metaHandlerCount mean ?
          Please add javadoc above.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java
          <https://reviews.apache.org/r/1421/#comment6404>

          Should default value be related to the handler count ?

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java
          <https://reviews.apache.org/r/1421/#comment6406>

          Write javadoc to describe algorithm.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java
          <https://reviews.apache.org/r/1421/#comment6405>

          Since 10 has special meaning, we should create constant for it.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment6407>

          Year isn't needed.

          • Ted

          On 2011-10-26 14:13:10, Jia Liu wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/1421/

          -----------------------------------------------------------

          (Updated 2011-10-26 14:13:10)

          Review request for hbase.

          Summary

          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.

          https://issues.apache.org/jira/browse/HBase-4120

          Diffs

          -----

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing

          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch

          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/#review2867 ----------------------------------------------------------- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6408 > No year please. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6409 > Remove extra 'the' http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6410 > Should read 'the priority map, cache the region, table and' http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java < https://reviews.apache.org/r/1421/#comment6399 > Year should be removed. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java < https://reviews.apache.org/r/1421/#comment6400 > PriorityHBaseServer is not abstract, right ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java < https://reviews.apache.org/r/1421/#comment6402 > Unless the line gets too wide, explanation would be on the same line as name of parameter. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java < https://reviews.apache.org/r/1421/#comment6403 > What does metaHandlerCount mean ? Please add javadoc above. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java < https://reviews.apache.org/r/1421/#comment6404 > Should default value be related to the handler count ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java < https://reviews.apache.org/r/1421/#comment6406 > Write javadoc to describe algorithm. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java < https://reviews.apache.org/r/1421/#comment6405 > Since 10 has special meaning, we should create constant for it. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment6407 > Year isn't needed. Ted On 2011-10-26 14:13:10, Jia Liu wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-10-26 14:13:10) Review request for hbase. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/#review2869
          -----------------------------------------------------------

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6416>

          I think special attributes should start with "_".

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6421>

          Javadoc should be added explaining the meaning of <key,value> for each of the maps.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6411>

          Class name is wrong.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6412>

          Default value here is different from the value on line 70.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6413>

          I don't think these exceptions can be ignored.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6414>

          Is getRefreshInterval a better name for this method ?

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6415>

          Should read 'priority should be between'.

          So only region priority can be negative ?

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6419>

          Would getTableActionPriorities be a better name for the method ?

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6418>

          I think contents of plus should be included in the message.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6417>

          This line can be omitted.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6420>

          Initiate should be initialize.

          • Ted

          On 2011-10-26 14:13:10, Jia Liu wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/1421/

          -----------------------------------------------------------

          (Updated 2011-10-26 14:13:10)

          Review request for hbase.

          Summary

          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.

          https://issues.apache.org/jira/browse/HBase-4120

          Diffs

          -----

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing

          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch

          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/#review2869 ----------------------------------------------------------- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6416 > I think special attributes should start with "_". http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6421 > Javadoc should be added explaining the meaning of <key,value> for each of the maps. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6411 > Class name is wrong. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6412 > Default value here is different from the value on line 70. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6413 > I don't think these exceptions can be ignored. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6414 > Is getRefreshInterval a better name for this method ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6415 > Should read 'priority should be between'. So only region priority can be negative ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6419 > Would getTableActionPriorities be a better name for the method ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6418 > I think contents of plus should be included in the message. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6417 > This line can be omitted. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6420 > Initiate should be initialize. Ted On 2011-10-26 14:13:10, Jia Liu wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-10-26 14:13:10) Review request for hbase. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-10-27 01:47:33.468611)

          Review request for hbase.

          Changes
          -------

          fix patch

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-10-27 01:47:33.468611) Review request for hbase. Changes ------- fix patch Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-10-27 01:57:57.358569)

          Review request for hbase.

          Changes
          -------

          fixed

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-10-27 01:57:57.358569) Review request for hbase. Changes ------- fixed Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-10-27 13:05:36.419396)

          Review request for hbase.

          Changes
          -------

          fix v3 patch's review problems

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-10-27 13:05:36.419396) Review request for hbase. Changes ------- fix v3 patch's review problems Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/#review2880
          -----------------------------------------------------------

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6449>

          This class doesn't belong to ipc module.
          How about lifting this class and PriorityJobQueue to org/apache/hadoop/hbase ?

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6450>

          This key should be named PRI_KEY_ACTION_PRIORITY. The string should be "_action_priority"

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6438>

          Should read 'The priority map caches the region's priority in memory.'

          Originally there was one javadoc for all the maps. Now this sentence should be specific.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6439>

          Should read 'Key is the region name which usually is obtained from the IPC call'

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6445>

          Under what circumstances would a region carry different priority than the table it belongs ?
          If there is such scenario, we should add a test for RegionPriority.
          Otherwise this map can be folded into tablePriMap.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6442>

          This map should be named scannerRegionMap

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6437>

          The variable name can be improved.
          How about naming it scannerPriMap ?

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6448>

          Class name is still incorrect, missing hbase component.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6447>

          Default value here is different from the value on line 70.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6436>

          Should read 'priority should be between'.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6435>

          Should read 'to set table action priority'

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6440>

          This variable should be named actionPriority

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6441>

          This variable should be called scannerId.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java
          <https://reviews.apache.org/r/1421/#comment6451>

          This parameter should be named priorityHandlerCount so that it is consistent with usage in HBaseServer.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment6446>

          Should read 'which have same or higher priority'

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java
          <https://reviews.apache.org/r/1421/#comment6444>

          Typo: should be setTablePriority

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java
          <https://reviews.apache.org/r/1421/#comment6443>

          Typo: should be setTablePriority

          • Ted

          On 2011-10-27 13:05:36, Jia Liu wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/1421/

          -----------------------------------------------------------

          (Updated 2011-10-27 13:05:36)

          Review request for hbase.

          Summary

          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.

          https://issues.apache.org/jira/browse/HBase-4120

          Diffs

          -----

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing

          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch

          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/#review2880 ----------------------------------------------------------- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6449 > This class doesn't belong to ipc module. How about lifting this class and PriorityJobQueue to org/apache/hadoop/hbase ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6450 > This key should be named PRI_KEY_ACTION_PRIORITY. The string should be "_action_priority" http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6438 > Should read 'The priority map caches the region's priority in memory.' Originally there was one javadoc for all the maps. Now this sentence should be specific. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6439 > Should read 'Key is the region name which usually is obtained from the IPC call' http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6445 > Under what circumstances would a region carry different priority than the table it belongs ? If there is such scenario, we should add a test for RegionPriority. Otherwise this map can be folded into tablePriMap. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6442 > This map should be named scannerRegionMap http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6437 > The variable name can be improved. How about naming it scannerPriMap ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6448 > Class name is still incorrect, missing hbase component. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6447 > Default value here is different from the value on line 70. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6436 > Should read 'priority should be between'. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6435 > Should read 'to set table action priority' http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6440 > This variable should be named actionPriority http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6441 > This variable should be called scannerId. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java < https://reviews.apache.org/r/1421/#comment6451 > This parameter should be named priorityHandlerCount so that it is consistent with usage in HBaseServer. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment6446 > Should read 'which have same or higher priority' http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java < https://reviews.apache.org/r/1421/#comment6444 > Typo: should be setTablePriority http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java < https://reviews.apache.org/r/1421/#comment6443 > Typo: should be setTablePriority Ted On 2011-10-27 13:05:36, Jia Liu wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-10-27 13:05:36) Review request for hbase. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          Ted Yu added a comment -

          Please take care of the following:

                // TODO Auto-generated method stub
          
          Show
          Ted Yu added a comment - Please take care of the following: // TODO Auto-generated method stub
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2011-10-27 17:18:08, Ted Yu wrote:

          >

          What about make Call implement the Writable interface? By this way, we can perform the class cast thing in qosFunction.apply() and make the PriorityJobQueue a more common class.

          But the PriorityFunction will also need to use HBaseServer.Call to convert Object to Call, then get the region name to find out the priority of this call.

          On 2011-10-27 17:18:08, Ted Yu wrote:

          > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java, line 47

          > <https://reviews.apache.org/r/1421/diff/9/?file=53796#file53796line47>

          >

          > This parameter should be named priorityHandlerCount so that it is consistent with usage in HBaseServer.

          In the WritableRpcEngine.Server, this parameter is called metaHandlerCount and in HBaseServer those handlers just process the calls relative to meta and root.

          So should I keep the same with HBaseServer or WritableRpcEngine.Server ?

          On 2011-10-27 17:18:08, Ted Yu wrote:

          > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java, line 59

          > <https://reviews.apache.org/r/1421/diff/9/?file=53795#file53795line59>

          >

          > This class doesn't belong to ipc module.

          > How about lifting this class and PriorityJobQueue to org/apache/hadoop/hbase ?

          Because this part of code use the Call class, move to org/apache/hadoop/hbase means this class must be change to public, is this OK?
          public void put(Object e) throws InterruptedException

          { HBaseServer.Call call = (HBaseServer.Call) (e); int pri = this.qosFunction.apply(call.param); this.add((T) call, pri); }

          What about make Call implement the Writable interface? By this way, we can perform the class cast thing in qosFunction.apply() and make the PriorityJobQueue a more common class.

          But the PriorityFunction will also need to use HBaseServer.Call to convert Object to Call, then get the region name to find out the priority of this call.

          • Jia

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/#review2880
          -----------------------------------------------------------

          On 2011-10-27 13:05:36, Jia Liu wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/1421/

          -----------------------------------------------------------

          (Updated 2011-10-27 13:05:36)

          Review request for hbase.

          Summary

          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.

          https://issues.apache.org/jira/browse/HBase-4120

          Diffs

          -----

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing

          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch

          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - On 2011-10-27 17:18:08, Ted Yu wrote: > What about make Call implement the Writable interface? By this way, we can perform the class cast thing in qosFunction.apply() and make the PriorityJobQueue a more common class. But the PriorityFunction will also need to use HBaseServer.Call to convert Object to Call, then get the region name to find out the priority of this call. On 2011-10-27 17:18:08, Ted Yu wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java , line 47 > < https://reviews.apache.org/r/1421/diff/9/?file=53796#file53796line47 > > > This parameter should be named priorityHandlerCount so that it is consistent with usage in HBaseServer. In the WritableRpcEngine.Server, this parameter is called metaHandlerCount and in HBaseServer those handlers just process the calls relative to meta and root. So should I keep the same with HBaseServer or WritableRpcEngine.Server ? On 2011-10-27 17:18:08, Ted Yu wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java , line 59 > < https://reviews.apache.org/r/1421/diff/9/?file=53795#file53795line59 > > > This class doesn't belong to ipc module. > How about lifting this class and PriorityJobQueue to org/apache/hadoop/hbase ? Because this part of code use the Call class, move to org/apache/hadoop/hbase means this class must be change to public, is this OK? public void put(Object e) throws InterruptedException { HBaseServer.Call call = (HBaseServer.Call) (e); int pri = this.qosFunction.apply(call.param); this.add((T) call, pri); } What about make Call implement the Writable interface? By this way, we can perform the class cast thing in qosFunction.apply() and make the PriorityJobQueue a more common class. But the PriorityFunction will also need to use HBaseServer.Call to convert Object to Call, then get the region name to find out the priority of this call. Jia ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/#review2880 ----------------------------------------------------------- On 2011-10-27 13:05:36, Jia Liu wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-10-27 13:05:36) Review request for hbase. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2011-10-26 21:54:57, Ted Yu wrote:

          > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java, line 131

          > <https://reviews.apache.org/r/1421/diff/6/?file=53369#file53369line131>

          >

          > Should read 'priority should be between'.

          >

          > So only region priority can be negative ?

          the priority user set must between 1~10,the negative priority is used by some system table like meta and root,or other urgency.
          If an IPC call is waiting too long, it's priority may increase to a negative number.

          • Jia

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/#review2869
          -----------------------------------------------------------

          On 2011-10-27 13:05:36, Jia Liu wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/1421/

          -----------------------------------------------------------

          (Updated 2011-10-27 13:05:36)

          Review request for hbase.

          Summary

          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.

          https://issues.apache.org/jira/browse/HBase-4120

          Diffs

          -----

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing

          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch

          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - On 2011-10-26 21:54:57, Ted Yu wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java , line 131 > < https://reviews.apache.org/r/1421/diff/6/?file=53369#file53369line131 > > > Should read 'priority should be between'. > > So only region priority can be negative ? the priority user set must between 1~10,the negative priority is used by some system table like meta and root,or other urgency. If an IPC call is waiting too long, it's priority may increase to a negative number. Jia ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/#review2869 ----------------------------------------------------------- On 2011-10-27 13:05:36, Jia Liu wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-10-27 13:05:36) Review request for hbase. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2011-08-09 14:39:49, Ted Yu wrote:

          > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java, line 534

          > <https://reviews.apache.org/r/1421/diff/3/?file=31597#file31597line534>

          >

          > Return value should be boolean.

          this class implements BlockingQueue and put is a override method..so I can't change the return value.

          • Jia

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/#review1345
          -----------------------------------------------------------

          On 2011-10-27 13:05:36, Jia Liu wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/1421/

          -----------------------------------------------------------

          (Updated 2011-10-27 13:05:36)

          Review request for hbase.

          Summary

          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.

          https://issues.apache.org/jira/browse/HBase-4120

          Diffs

          -----

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing

          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch

          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - On 2011-08-09 14:39:49, Ted Yu wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java , line 534 > < https://reviews.apache.org/r/1421/diff/3/?file=31597#file31597line534 > > > Return value should be boolean. this class implements BlockingQueue and put is a override method..so I can't change the return value. Jia ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/#review1345 ----------------------------------------------------------- On 2011-10-27 13:05:36, Jia Liu wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-10-27 13:05:36) Review request for hbase. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-10-28 13:45:51.987878)

          Review request for hbase.

          Changes
          -------

          fix bugs of patch-v4

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-10-28 13:45:51.987878) Review request for hbase. Changes ------- fix bugs of patch-v4 Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          Ted Yu added a comment -

          For metaHandlerCount parameter, we can keep its name since it is for .META. and ROOT regions.

          For now, we can keep PriorityFunction.java in ipc module.

          Show
          Ted Yu added a comment - For metaHandlerCount parameter, we can keep its name since it is for .META. and ROOT regions. For now, we can keep PriorityFunction.java in ipc module.
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-10-31 05:22:05.235409)

          Review request for hbase.

          Changes
          -------

          rewrite test cases.

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-10-31 05:22:05.235409) Review request for hbase. Changes ------- rewrite test cases. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/#review2967
          -----------------------------------------------------------

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6626>

          Should we return from this method ?
          list would be null.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6625>

          list may be null if IOException was encountered above.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment6627>

          Should read 'Refresh priorities'

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java
          <https://reviews.apache.org/r/1421/#comment6628>

          Should read 'priorities of threads range from'

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment6629>

          Can you explain the relationship between move and handleFreshInter ?

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment6631>

          Please add space around =.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment6630>

          hbase.schedule.refresh.interval should be a better name for this parameter.

          If user specifies a large value, say 100, move would be negative. Is that acceptable ?

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment6632>

          We'd better change lowestThreadPriority to a constant - we're not using value for the passed lowestThreadPriority

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment6635>

          A better name for the method would be refreshPeriodically().

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment6634>

          This was for debugging purpose.
          Can we remove this ?

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment6633>

          Why don't we respond to runtime exceptions ?

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment6636>

          This method isn't called anywhere.

          • Ted

          On 2011-10-31 05:22:05, Jia Liu wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/1421/

          -----------------------------------------------------------

          (Updated 2011-10-31 05:22:05)

          Review request for hbase.

          Summary

          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.

          https://issues.apache.org/jira/browse/HBase-4120

          Diffs

          -----

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing

          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch

          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/#review2967 ----------------------------------------------------------- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6626 > Should we return from this method ? list would be null. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6625 > list may be null if IOException was encountered above. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment6627 > Should read 'Refresh priorities' http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java < https://reviews.apache.org/r/1421/#comment6628 > Should read 'priorities of threads range from' http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment6629 > Can you explain the relationship between move and handleFreshInter ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment6631 > Please add space around =. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment6630 > hbase.schedule.refresh.interval should be a better name for this parameter. If user specifies a large value, say 100, move would be negative. Is that acceptable ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment6632 > We'd better change lowestThreadPriority to a constant - we're not using value for the passed lowestThreadPriority http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment6635 > A better name for the method would be refreshPeriodically(). http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment6634 > This was for debugging purpose. Can we remove this ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment6633 > Why don't we respond to runtime exceptions ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment6636 > This method isn't called anywhere. Ted On 2011-10-31 05:22:05, Jia Liu wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-10-31 05:22:05) Review request for hbase. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-11-02 03:32:00.146288)

          Review request for hbase.

          Changes
          -------

          Fix the test case's bugs, all tests passed in maven.

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-11-02 03:32:00.146288) Review request for hbase. Changes ------- Fix the test case's bugs, all tests passed in maven. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestForTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          Ted Yu added a comment -

          Here is some background for the current implementation provided by Liu Jia:

          We have moved the main functionality of table priority (HBASE-4120) to PriorityFunction and PriorityJobQueue.
          PriorityFunction implements Function<Writable, Integer> and is used by the PriorityJobQueue to decide the priority of each IPC call.
          Through this we make the PriorityHBaseServer much simpler.

          In current implementation of HBase, there would be one priority for each method:

                for (Method m : HRegionServer.class.getMethods()) {
                  QosPriority p = m.getAnnotation(QosPriority.class);
          

          But in our implementation, method can have different priorities for different HBase tables.
          e.g. for tables A and B in the same cluster, table A serves reads mostly while table B serves writes more often.
          we may want to give high priority to get() for table A while giving low priority to put(). For table B, we may want low priority for get() and high priority for put().

          That's why we introduced ActionPrioriy for the methods (such as get and put). At the same time, tables have their base priorities.
          We believe the above approach is more flexible.

          We do not interfere with the current Qos Function,which handles the meta and root request respectively.
          but in the future a more generalized approach is better.

          Show
          Ted Yu added a comment - Here is some background for the current implementation provided by Liu Jia: We have moved the main functionality of table priority ( HBASE-4120 ) to PriorityFunction and PriorityJobQueue. PriorityFunction implements Function<Writable, Integer> and is used by the PriorityJobQueue to decide the priority of each IPC call. Through this we make the PriorityHBaseServer much simpler. In current implementation of HBase, there would be one priority for each method: for (Method m : HRegionServer.class.getMethods()) { QosPriority p = m.getAnnotation(QosPriority.class); But in our implementation, method can have different priorities for different HBase tables. e.g. for tables A and B in the same cluster, table A serves reads mostly while table B serves writes more often. we may want to give high priority to get() for table A while giving low priority to put(). For table B, we may want low priority for get() and high priority for put(). That's why we introduced ActionPrioriy for the methods (such as get and put). At the same time, tables have their base priorities. We believe the above approach is more flexible. We do not interfere with the current Qos Function,which handles the meta and root request respectively. but in the future a more generalized approach is better.
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-11-03 11:42:29.986508)

          Review request for hbase.

          Changes
          -------

          Make test cases cost less time.

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-11-03 11:42:29.986508) Review request for hbase. Changes ------- Make test cases cost less time. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          Liu Jia added a comment -

          table priority patch for trunk.

          Show
          Liu Jia added a comment - table priority patch for trunk.
          Hide
          Ted Yu added a comment -

          In Performance_of_Table_priority.pdf attached to this JIRA, you can see performance charts illustrating the benefits of table prioritization.

          From Liu Jia:
          After we finished the HBase_isolation_and_allocation_user_guide.pdf there were about twelve servers in the cluster,
          and from the screen shots in the HBase_isolation_and_allocation_user_guide.pdf you can see there are at least 700 regions on region server.
          Prior to that we had two engineers working on this functionality for about 1.5 month.

          Most of time, there are at least three projects running on this cluster. Applications can share the cluster and run concurrently.
          Some of the aplications like TaoBao's data cube has about 1TB data per month and another application (user behavior tracking) has about 800GB data per day but we only keep two days' data. The number of write requests is about 30000 per second,with record size of about 70B. These two aplications had run on this cluster separately
          for at least one month.

          Show
          Ted Yu added a comment - In Performance_of_Table_priority.pdf attached to this JIRA, you can see performance charts illustrating the benefits of table prioritization. From Liu Jia: After we finished the HBase_isolation_and_allocation_user_guide.pdf there were about twelve servers in the cluster, and from the screen shots in the HBase_isolation_and_allocation_user_guide.pdf you can see there are at least 700 regions on region server. Prior to that we had two engineers working on this functionality for about 1.5 month. Most of time, there are at least three projects running on this cluster. Applications can share the cluster and run concurrently. Some of the aplications like TaoBao's data cube has about 1TB data per month and another application (user behavior tracking) has about 800GB data per day but we only keep two days' data. The number of write requests is about 30000 per second,with record size of about 70B. These two aplications had run on this cluster separately for at least one month.
          Hide
          Ted Yu added a comment -

          Patch from review board.

          Show
          Ted Yu added a comment - Patch from review board.
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-11-22 06:06:45.314957)

          Review request for hbase.

          Changes
          -------

          increase the thread number of test cases.

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-11-22 06:06:45.314957) Review request for hbase. Changes ------- increase the thread number of test cases. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          Liu Jia added a comment -

          increase the thread number in test cases

          Show
          Liu Jia added a comment - increase the thread number in test cases
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/#review3444
          -----------------------------------------------------------

          initial comments. haven't had time to look through the whole diff in detail. I'm guessing this was made against 0.90? This is missing integration with a lot of new functionality added a while ago into 92.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java
          <https://reviews.apache.org/r/1421/#comment7687>

          should this functionality not be disabled by default?

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment7696>

          this pull model seems a little hacky. why can't we just push this information when a region comes online on this server? we have online schema updates, so we can update the table priority with minimal downtime.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment7694>

          this pull model seems a little hacky. why can't we just push this information when a region comes online on this server? we have online schema updates, so we can update the table priority with minimal downtime.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment7688>

          instead of this big switch statement, why don't you add a getPriority() function to the Operation base class? All database operations (gets, puts, deletes) are supposed to override this anyways do we can print out fingerprint and other debug information. You can see its use in WritableRpcEngine.java

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment7689>

          your intent is to lower the priority of multiputs, correct? I see that HIGHEST_PRI = -10. If a MultiAction has that priority, won't it make a MultiAction<T> have a higher priority than T?

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment7690>

          again, I'm confused about why we need to contact the master to get this information. we only care about the regions that are online for this server, correct?

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java
          <https://reviews.apache.org/r/1421/#comment7691>

          can you explain this more? what does this mapping look like? In other words

          thread pri 1-10 == system pri X-Y

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment7700>

          why are all these accessors needed? Why can't you just use

          if(this.queue.size() < this.capacity) queueFull.signal()

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment7698>

          this should be signal() instead of signalAll(). You will only decrease capacity by 1, so you should only wake 1 thread.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment7692>

          so, if you've waited too long, you'll add the Job to the queue anyways? it's not a tryAdd() then, which would imply that the action could fail. It's just a normal add()

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment7699>

          there is a race condition between (size >= capacity) and addLock. Shouldn't you test the condition again after getting the lock?

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java
          <https://reviews.apache.org/r/1421/#comment7701>

          LOG.debug

          • Nicolas

          On 2011-11-22 06:06:45, Jia Liu wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/1421/

          -----------------------------------------------------------

          (Updated 2011-11-22 06:06:45)

          Review request for hbase.

          Summary

          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.

          https://issues.apache.org/jira/browse/HBase-4120

          Diffs

          -----

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestActionPriority.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestPriorityJobQueue.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing

          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch

          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/#review3444 ----------------------------------------------------------- initial comments. haven't had time to look through the whole diff in detail. I'm guessing this was made against 0.90? This is missing integration with a lot of new functionality added a while ago into 92. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java < https://reviews.apache.org/r/1421/#comment7687 > should this functionality not be disabled by default? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment7696 > this pull model seems a little hacky. why can't we just push this information when a region comes online on this server? we have online schema updates, so we can update the table priority with minimal downtime. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment7694 > this pull model seems a little hacky. why can't we just push this information when a region comes online on this server? we have online schema updates, so we can update the table priority with minimal downtime. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment7688 > instead of this big switch statement, why don't you add a getPriority() function to the Operation base class? All database operations (gets, puts, deletes) are supposed to override this anyways do we can print out fingerprint and other debug information. You can see its use in WritableRpcEngine.java http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment7689 > your intent is to lower the priority of multiputs, correct? I see that HIGHEST_PRI = -10. If a MultiAction has that priority, won't it make a MultiAction<T> have a higher priority than T? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment7690 > again, I'm confused about why we need to contact the master to get this information. we only care about the regions that are online for this server, correct? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java < https://reviews.apache.org/r/1421/#comment7691 > can you explain this more? what does this mapping look like? In other words thread pri 1-10 == system pri X-Y http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment7700 > why are all these accessors needed? Why can't you just use if(this.queue.size() < this.capacity) queueFull.signal() http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment7698 > this should be signal() instead of signalAll(). You will only decrease capacity by 1, so you should only wake 1 thread. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment7692 > so, if you've waited too long, you'll add the Job to the queue anyways? it's not a tryAdd() then, which would imply that the action could fail. It's just a normal add() http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment7699 > there is a race condition between (size >= capacity) and addLock. Shouldn't you test the condition again after getting the lock? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java < https://reviews.apache.org/r/1421/#comment7701 > LOG.debug Nicolas On 2011-11-22 06:06:45, Jia Liu wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-11-22 06:06:45) Review request for hbase. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          Ted Yu added a comment -

          @Nicolas:
          Thanks for your valuable review.

          This feature was developed independently of HBASE-1730/HBASE-4213.
          Do you think priority refreshing can be done in a separate JIRA ?

          Show
          Ted Yu added a comment - @Nicolas: Thanks for your valuable review. This feature was developed independently of HBASE-1730 / HBASE-4213 . Do you think priority refreshing can be done in a separate JIRA ?
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2011-11-22 23:35:06, Nicolas Spiegelberg wrote:

          > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java, line 224

          > <https://reviews.apache.org/r/1421/diff/14/?file=59672#file59672line224>

          >

          > this pull model seems a little hacky. why can't we just push this information when a region comes online on this server? we have online schema updates, so we can update the table priority with minimal downtime.

          It's a good idea to change the pull model to push,what about add a hook in HRegionServer public void addToOnlineRegions(HRegion region) to initialize the priority of regions?
          I will try this in next patch.

          On 2011-11-22 23:35:06, Nicolas Spiegelberg wrote:

          > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java, lines 89-104

          > <https://reviews.apache.org/r/1421/diff/14/?file=59674#file59674line89>

          >

          > why are all these accessors needed? Why can't you just use

          >

          > if(this.queue.size() < this.capacity) queueFull.signal()

          Changed in new patch.

          On 2011-11-22 23:35:06, Nicolas Spiegelberg wrote:

          > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java, lines 472-473

          > <https://reviews.apache.org/r/1421/diff/14/?file=59672#file59672line472>

          >

          > can you explain this more? what does this mapping look like? In other words

          >

          > thread pri 1-10 == system pri X-Y

          I add some explanation to the method.
          Translate thread priority to system priority, thread priority is from 1 to 10 and 10 is highest priority,
          but system priority used by rpc is the same with linux system,
          small number means high priority.
          So translate the thread priority to system priority to see which job this thread can handle.

          On 2011-11-22 23:35:06, Nicolas Spiegelberg wrote:

          > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java, lines 321-329

          > <https://reviews.apache.org/r/1421/diff/14/?file=59672#file59672line321>

          >

          > your intent is to lower the priority of multiputs, correct? I see that HIGHEST_PRI = -10. If a MultiAction has that priority, won't it make a MultiAction<T> have a higher priority than T?

          MultiAction is the same with single ones, the priority = priority of region + priority of action. this part of code is to find what kind of action in this MultiAction.

          • Jia

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/#review3444
          -----------------------------------------------------------

          On 2011-11-22 06:06:45, Jia Liu wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/1421/

          -----------------------------------------------------------

          (Updated 2011-11-22 06:06:45)

          Review request for hbase.

          Summary

          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.

          https://issues.apache.org/jira/browse/HBase-4120

          Diffs

          -----

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestActionPriority.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestPriorityJobQueue.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing

          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch

          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - On 2011-11-22 23:35:06, Nicolas Spiegelberg wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java , line 224 > < https://reviews.apache.org/r/1421/diff/14/?file=59672#file59672line224 > > > this pull model seems a little hacky. why can't we just push this information when a region comes online on this server? we have online schema updates, so we can update the table priority with minimal downtime. It's a good idea to change the pull model to push,what about add a hook in HRegionServer public void addToOnlineRegions(HRegion region) to initialize the priority of regions? I will try this in next patch. On 2011-11-22 23:35:06, Nicolas Spiegelberg wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java , lines 89-104 > < https://reviews.apache.org/r/1421/diff/14/?file=59674#file59674line89 > > > why are all these accessors needed? Why can't you just use > > if(this.queue.size() < this.capacity) queueFull.signal() Changed in new patch. On 2011-11-22 23:35:06, Nicolas Spiegelberg wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java , lines 472-473 > < https://reviews.apache.org/r/1421/diff/14/?file=59672#file59672line472 > > > can you explain this more? what does this mapping look like? In other words > > thread pri 1-10 == system pri X-Y I add some explanation to the method. Translate thread priority to system priority, thread priority is from 1 to 10 and 10 is highest priority, but system priority used by rpc is the same with linux system, small number means high priority. So translate the thread priority to system priority to see which job this thread can handle. On 2011-11-22 23:35:06, Nicolas Spiegelberg wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java , lines 321-329 > < https://reviews.apache.org/r/1421/diff/14/?file=59672#file59672line321 > > > your intent is to lower the priority of multiputs, correct? I see that HIGHEST_PRI = -10. If a MultiAction has that priority, won't it make a MultiAction<T> have a higher priority than T? MultiAction is the same with single ones, the priority = priority of region + priority of action. this part of code is to find what kind of action in this MultiAction. Jia ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/#review3444 ----------------------------------------------------------- On 2011-11-22 06:06:45, Jia Liu wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-11-22 06:06:45) Review request for hbase. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-11-28 11:24:06.007647)

          Review request for hbase.

          Changes
          -------

          Change the region priority refresh mode from pull to push, fix code to past some core tests.

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Get.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiPut.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Mutation.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Operation.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Scan.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1207069
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-11-28 11:24:06.007647) Review request for hbase. Changes ------- Change the region priority refresh mode from pull to push, fix code to past some core tests. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Get.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiPut.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Mutation.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Operation.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Scan.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1207069 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2011-11-22 23:35:06, Nicolas Spiegelberg wrote:

          > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java, line 441

          > <https://reviews.apache.org/r/1421/diff/14/?file=59672#file59672line441>

          >

          > again, I'm confused about why we need to contact the master to get this information. we only care about the regions that are online for this server, correct?

          In new patch,this part of code is removed.

          • Jia

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/#review3444
          -----------------------------------------------------------

          On 2011-11-28 11:24:06, Jia Liu wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/1421/

          -----------------------------------------------------------

          (Updated 2011-11-28 11:24:06)

          Review request for hbase.

          Summary

          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.

          https://issues.apache.org/jira/browse/HBase-4120

          Diffs

          -----

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Get.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiPut.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Mutation.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Operation.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Scan.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1207069

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestActionPriority.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestPriorityJobQueue.java PRE-CREATION

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing

          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch

          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - On 2011-11-22 23:35:06, Nicolas Spiegelberg wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java , line 441 > < https://reviews.apache.org/r/1421/diff/14/?file=59672#file59672line441 > > > again, I'm confused about why we need to contact the master to get this information. we only care about the regions that are online for this server, correct? In new patch,this part of code is removed. Jia ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/#review3444 ----------------------------------------------------------- On 2011-11-28 11:24:06, Jia Liu wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-11-28 11:24:06) Review request for hbase. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Get.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiPut.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Mutation.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Operation.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Scan.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1189169 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1207069 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          Liu Jia added a comment -

          Change the region priority refresh mode from pull to push, fix code to past some core tests.

          Show
          Liu Jia added a comment - Change the region priority refresh mode from pull to push, fix code to past some core tests.
          Hide
          Ted Yu added a comment -

          The new tests should be placed under src/test/java/org/apache/hadoop/hbase/allocation/, instead of src/test/java/org/apache/hadoop/hbase/allocation/test

          Show
          Ted Yu added a comment - The new tests should be placed under src/test/java/org/apache/hadoop/hbase/allocation/, instead of src/test/java/org/apache/hadoop/hbase/allocation/test
          Hide
          Ted Yu added a comment -

          Please also categorize the new tests.
          See N's email on dev@hbase.apache.org, entitled 'unit tests - pom.xml & surefire changed - categories are available', for guideline.

          Show
          Ted Yu added a comment - Please also categorize the new tests. See N's email on dev@hbase.apache.org, entitled 'unit tests - pom.xml & surefire changed - categories are available', for guideline.
          Hide
          Ted Yu added a comment -

          Indentation in certain classes was off.
          e.g.

          public static int initRegionPriority(HRegion region) {
          

          Please use Eclipse formatter Nicloas attached to HBASE-3678 to format every file.

          private static int initRegionPriority(String region, boolean force) {
          ...
                if (ret != null)
                  return ret;
          

          Please enclose the return statement in curly braces.

          Show
          Ted Yu added a comment - Indentation in certain classes was off. e.g. public static int initRegionPriority(HRegion region) { Please use Eclipse formatter Nicloas attached to HBASE-3678 to format every file. private static int initRegionPriority( String region, boolean force) { ... if (ret != null ) return ret; Please enclose the return statement in curly braces.
          Hide
          Ted Yu added a comment -

          In PriorityJobQueue.java:

              public void setPutPlus(int putPlus) {
          

          Maybe putPlus supersedes previous version of this feature at your company. But for other HBase users, such name may be confusing.
          I think the method can be named setPutPriority and parameter can be named priority.

          Please remove plus from method and parameter names in PriorityJobQueue.java

          For Mutation.java, I see:

            public int getPriority(ActionPriorities actionPriority) {
          	  return actionPriority.getGetPlus();
            }
          

          Is getPutPriority() be called above ? All mutations involve some kind of write.

          Show
          Ted Yu added a comment - In PriorityJobQueue.java: public void setPutPlus( int putPlus) { Maybe putPlus supersedes previous version of this feature at your company. But for other HBase users, such name may be confusing. I think the method can be named setPutPriority and parameter can be named priority. Please remove plus from method and parameter names in PriorityJobQueue.java For Mutation.java, I see: public int getPriority(ActionPriorities actionPriority) { return actionPriority.getGetPlus(); } Is getPutPriority() be called above ? All mutations involve some kind of write.
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-11-29 06:06:22.278826)

          Review request for hbase.

          Changes
          -------

          code formated

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Get.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiPut.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Mutation.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Operation.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Scan.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-11-29 06:06:22.278826) Review request for hbase. Changes ------- code formated Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Get.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiPut.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Mutation.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Operation.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Scan.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/allocation/test/TestTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-11-30 09:05:27.762350)

          Review request for hbase.

          Changes
          -------

          Add category to test cases.

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Get.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiPut.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Mutation.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Operation.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Scan.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-11-30 09:05:27.762350) Review request for hbase. Changes ------- Add category to test cases. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Get.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiPut.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Mutation.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Operation.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Scan.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-12-01 02:59:26.539791)

          Review request for hbase.

          Changes
          -------

          In Mutation: return actionPriority.getPutPriority(); instead of getGetPriority

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Get.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiPut.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Mutation.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Operation.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Scan.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-12-01 02:59:26.539791) Review request for hbase. Changes ------- In Mutation: return actionPriority.getPutPriority(); instead of getGetPriority Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Get.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiPut.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Mutation.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Operation.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Scan.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-12-06 03:31:52.868365)

          Review request for hbase.

          Changes
          -------

          Make the test cases more stable, delete old tables first.

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1210064
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Get.java 1210064
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiPut.java 1210064
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Mutation.java 1210064
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Operation.java 1210064
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Scan.java 1210064
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1210064
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1210064
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1210064
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-12-06 03:31:52.868365) Review request for hbase. Changes ------- Make the test cases more stable, delete old tables first. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1210064 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Get.java 1210064 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiPut.java 1210064 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Mutation.java 1210064 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Operation.java 1210064 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Scan.java 1210064 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1210064 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1210064 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1210064 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          Liu Jia added a comment -

          Test cases are more stable in this patch.

          Show
          Liu Jia added a comment - Test cases are more stable in this patch.
          Hide
          Liu Jia added a comment -

          Test cases are more stable in this patch.

          Show
          Liu Jia added a comment - Test cases are more stable in this patch.
          Hide
          Ted Yu added a comment -

          From Andrew Purtell (see 'trip report for Hadoop In China' up on dev@):

          After 0.92 is out I intend to champion / mentor / co-develop 4120 and the follow on table allocation work and target 0.94 for it. I think the RPC QoS aspect is not too controversial. The allocation/reservation aspects I'd like to aim for a coprocessor or at least master plugin based integration so they won't impact stability for users who don't enable it. Unlike RPC QoS I suspect the changes needed to core can be minimized to coprocessor framework additions. Follow up in new JIRAs soon.

          Show
          Ted Yu added a comment - From Andrew Purtell (see 'trip report for Hadoop In China' up on dev@): After 0.92 is out I intend to champion / mentor / co-develop 4120 and the follow on table allocation work and target 0.94 for it. I think the RPC QoS aspect is not too controversial. The allocation/reservation aspects I'd like to aim for a coprocessor or at least master plugin based integration so they won't impact stability for users who don't enable it. Unlike RPC QoS I suspect the changes needed to core can be minimized to coprocessor framework additions. Follow up in new JIRAs soon.
          Hide
          Ted Yu added a comment -

          Andy suggested placing the PriorityFunction.initRegionPriority(region) call in RegionObserver.postOpen()

          Show
          Ted Yu added a comment - Andy suggested placing the PriorityFunction.initRegionPriority(region) call in RegionObserver.postOpen()
          Hide
          Liu Jia added a comment -

          @Ted:
          Why doesn't use close() of HRegionServer to close expired scanner in the inner class ScannerListener?
          If HRegionServer.close() is called by ScannerListener, I can just use RegionObserver.preScannerClose() to deal with the invalid scanners.

          Show
          Liu Jia added a comment - @Ted: Why doesn't use close() of HRegionServer to close expired scanner in the inner class ScannerListener? If HRegionServer.close() is called by ScannerListener, I can just use RegionObserver.preScannerClose() to deal with the invalid scanners.
          Hide
          Ted Yu added a comment -

          But preScannerClose() is only passed one scanner.

          Show
          Ted Yu added a comment - But preScannerClose() is only passed one scanner.
          Hide
          Liu Jia added a comment -
          ScannerListener.java
          //or the following code should hook to preScannerClose(),so I don't need to add this line   "PriorityFunction.removeScanner(this.scannerName);"
            private class ScannerListener implements LeaseListener {
              private final String scannerName;
          
              ScannerListener(final String n) {
                this.scannerName = n;
              }
          
              public void leaseExpired() {
                LOG.info("Scanner " + this.scannerName + " lease expired");
                RegionScanner s = scanners.remove(this.scannerName);
                if (s != null) {
                  try {
                    s.close();
                  } catch (IOException e) {
                    LOG.error("Closing scanner", e);
                  }
                }
                if (enablePriority) {
                  PriorityFunction.removeScanner(this.scannerName);
                }
              }
            }
          
          Show
          Liu Jia added a comment - ScannerListener.java //or the following code should hook to preScannerClose(),so I don't need to add this line "PriorityFunction.removeScanner( this .scannerName);" private class ScannerListener implements LeaseListener { private final String scannerName; ScannerListener( final String n) { this .scannerName = n; } public void leaseExpired() { LOG.info( "Scanner " + this .scannerName + " lease expired" ); RegionScanner s = scanners.remove( this .scannerName); if (s != null ) { try { s.close(); } catch (IOException e) { LOG.error( "Closing scanner" , e); } } if (enablePriority) { PriorityFunction.removeScanner( this .scannerName); } } }
          Hide
          Ted Yu added a comment -

          Looks like we should expose scanner lease expiration through new coprocessor API.

          Show
          Ted Yu added a comment - Looks like we should expose scanner lease expiration through new coprocessor API.
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-12-08 09:41:50.209106)

          Review request for hbase.

          Changes
          -------

          The coprocessor version of this patch.

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Get.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiPut.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Mutation.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Operation.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Scan.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/QosRegionObserver.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1207113
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-12-08 09:41:50.209106) Review request for hbase. Changes ------- The coprocessor version of this patch. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Get.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiPut.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Mutation.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Operation.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Scan.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/QosRegionObserver.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1207113 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          Ted Yu added a comment -

          Please add license to QosRegionObserver.java

          I think we should enhance ScannerListener.leaseExpired() with pre/postScannerClose() so that features such as table priority can receive consistent notification.
          The trick here is that RegionScanner only exposes HRegionInfo. We should be able to utilize onlineRegions in looking up HRegion by region name.
          Then we should be able to call the following:

                  if (region != null && region.getCoprocessorHost() != null) {
                    region.getCoprocessorHost().postScannerClose(s);
                  }
          
          Show
          Ted Yu added a comment - Please add license to QosRegionObserver.java I think we should enhance ScannerListener.leaseExpired() with pre/postScannerClose() so that features such as table priority can receive consistent notification. The trick here is that RegionScanner only exposes HRegionInfo. We should be able to utilize onlineRegions in looking up HRegion by region name. Then we should be able to call the following: if (region != null && region.getCoprocessorHost() != null ) { region.getCoprocessorHost().postScannerClose(s); }
          Hide
          Ted Yu added a comment -

          In PriorityFunction.java, several lines are much wider than 80 characters.
          Please use the formatter from HBASE-3678 on the new files.

          Show
          Ted Yu added a comment - In PriorityFunction.java, several lines are much wider than 80 characters. Please use the formatter from HBASE-3678 on the new files.
          Hide
          Ted Yu added a comment -

          Here is proposed change to HRegionServer:

          Index: src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
          ===================================================================
          --- src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java	(revision 1212433)
          +++ src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java	(working copy)
          @@ -2353,9 +2353,17 @@
                 RegionScanner s = scanners.remove(this.scannerName);
                 if (s != null) {
                   try {
          +          HRegion region = getRegion(s.getRegionInfo().getRegionName());
          +          if (region != null && region.getCoprocessorHost() != null) {
          +            region.getCoprocessorHost().preScannerClose(s);
          +          }
                     s.close();
          +          if (region != null && region.getCoprocessorHost() != null) {
          +            region.getCoprocessorHost().postScannerClose(s);
          +          }
                   } catch (IOException e) {
          -          LOG.error("Closing scanner", e);
          +          LOG.error("Closing scanner for " + 
          +              s.getRegionInfo().getRegionNameAsString(), e);
                   }
                 }
               }
          
          Show
          Ted Yu added a comment - Here is proposed change to HRegionServer: Index: src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java (revision 1212433) +++ src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java (working copy) @@ -2353,9 +2353,17 @@ RegionScanner s = scanners.remove( this .scannerName); if (s != null ) { try { + HRegion region = getRegion(s.getRegionInfo().getRegionName()); + if (region != null && region.getCoprocessorHost() != null ) { + region.getCoprocessorHost().preScannerClose(s); + } s.close(); + if (region != null && region.getCoprocessorHost() != null ) { + region.getCoprocessorHost().postScannerClose(s); + } } catch (IOException e) { - LOG.error( "Closing scanner" , e); + LOG.error( "Closing scanner for " + + s.getRegionInfo().getRegionNameAsString(), e); } } }
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-12-12 03:43:32.270550)

          Review request for hbase.

          Changes
          -------

          Move all Operation.getPriority() methods to PriorityJobQueue.ActionPriorities.
          Move all Qos related codes out of HRegionServer.
          Add coprocessor hook to HRegionServer.ScannerListener.leaseExpired().
          Improve the TestTablePriority class to test scan and put operations by default.

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1213130
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1213130
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/QosRegionObserver.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1213130
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-12-12 03:43:32.270550) Review request for hbase. Changes ------- Move all Operation.getPriority() methods to PriorityJobQueue.ActionPriorities. Move all Qos related codes out of HRegionServer. Add coprocessor hook to HRegionServer.ScannerListener.leaseExpired(). Improve the TestTablePriority class to test scan and put operations by default. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1213130 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1213130 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/QosRegionObserver.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1213130 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-12-13 09:20:33.777262)

          Review request for hbase.

          Changes
          -------

          Table priority is disabled by default, to enable it you should add the following entry to your configuration.

          <property>
          <name>hbase.tablepriority.enable</name>
          <value>true</value>
          </property>

          If table priority is enabled by default, the following two unit tests will fail.

          Failed tests:
          testExceptionFromCoprocessorWhenCreatingTable(org.apache.hadoop.hbase.coprocessor.TestMasterCoprocessorExceptionWithAbort)
          testExceptionFromCoprocessorWhenCreatingTable(org.apache.hadoop.hbase.coprocessor.TestMasterCoprocessorExceptionWithRemove)

          assertTrue(master.getLoadedCoprocessors().
          equals("[" +
          TestMasterCoprocessorExceptionWithAbort.BuggyMasterObserver.class.getName() +
          "]"));
          String coprocessorName =
          BuggyMasterObserver.class.getName();
          assertTrue(master.getLoadedCoprocessors().equals("[" + coprocessorName + "]"));

          If enabled the table priority, org.apache.hadoop.hbase.ipc.QosRegionObserver will be loaded and this two assertions couldn't figure out what is in the LoadedCoprocessors.

          [org.apache.hadoop.hbase.ipc.QosRegionObserver, org.apache.hadoop.hbase.coprocessor.TestMasterCoprocessorExceptionWithRemove$BuggyMasterObserver]

          Perhaps we could make some changes like this:

          assertTrue(master.getLoadedCoprocessors().indexOf
          (TestMasterCoprocessorExceptionWithAbort.BuggyMasterObserver.class.getName())!=-1);

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1213130
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1213130
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/QosRegionObserver.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1213130
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-12-13 09:20:33.777262) Review request for hbase. Changes ------- Table priority is disabled by default, to enable it you should add the following entry to your configuration. <property> <name>hbase.tablepriority.enable</name> <value>true</value> </property> If table priority is enabled by default, the following two unit tests will fail. Failed tests: testExceptionFromCoprocessorWhenCreatingTable(org.apache.hadoop.hbase.coprocessor.TestMasterCoprocessorExceptionWithAbort) testExceptionFromCoprocessorWhenCreatingTable(org.apache.hadoop.hbase.coprocessor.TestMasterCoprocessorExceptionWithRemove) assertTrue(master.getLoadedCoprocessors(). equals("[" + TestMasterCoprocessorExceptionWithAbort.BuggyMasterObserver.class.getName() + "]")); String coprocessorName = BuggyMasterObserver.class.getName(); assertTrue(master.getLoadedCoprocessors().equals(" [" + coprocessorName + "] ")); If enabled the table priority, org.apache.hadoop.hbase.ipc.QosRegionObserver will be loaded and this two assertions couldn't figure out what is in the LoadedCoprocessors. [org.apache.hadoop.hbase.ipc.QosRegionObserver, org.apache.hadoop.hbase.coprocessor.TestMasterCoprocessorExceptionWithRemove$BuggyMasterObserver] Perhaps we could make some changes like this: assertTrue(master.getLoadedCoprocessors().indexOf (TestMasterCoprocessorExceptionWithAbort.BuggyMasterObserver.class.getName())!=-1); Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1213130 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1213130 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/QosRegionObserver.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1213130 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          Ted Yu added a comment -

          I think the two failed tests should be modified.
          There would be more coprocessors loaded in the future. It is desirable to make the assertions deterministic.

          Show
          Ted Yu added a comment - I think the two failed tests should be modified. There would be more coprocessors loaded in the future. It is desirable to make the assertions deterministic.
          Hide
          Ted Yu added a comment -

          In PriorityJobQueue, the call to addLock.unlock() should be wrapped in finally block.

          Show
          Ted Yu added a comment - In PriorityJobQueue, the call to addLock.unlock() should be wrapped in finally block.
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-12-14 17:12:58.306061)

          Review request for hbase.

          Changes
          -------

          Modify the two test cases :org.apache.hadoop.hbase.coprocessor.TestMasterCoprocessorExceptionWithAbort
          org.apache.hadoop.hbase.coprocessor.TestMasterCoprocessorExceptionWithRemove
          Enable the table priority function by default.

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1213130
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1213130
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/QosRegionObserver.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1213130
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterCoprocessorExceptionWithAbort.java 1213130
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterCoprocessorExceptionWithRemove.java 1213130
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriority.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-12-14 17:12:58.306061) Review request for hbase. Changes ------- Modify the two test cases :org.apache.hadoop.hbase.coprocessor.TestMasterCoprocessorExceptionWithAbort org.apache.hadoop.hbase.coprocessor.TestMasterCoprocessorExceptionWithRemove Enable the table priority function by default. Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1213130 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1213130 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/QosRegionObserver.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1213130 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterCoprocessorExceptionWithAbort.java 1213130 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterCoprocessorExceptionWithRemove.java 1213130 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriority.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          Liu Jia added a comment -

          Modify the two test cases :org.apache.hadoop.hbase.coprocessor.TestMasterCoprocessorExceptionWithAbort
          org.apache.hadoop.hbase.coprocessor.TestMasterCoprocessorExceptionWithRemove
          Enable the table priority function by default.

          Show
          Liu Jia added a comment - Modify the two test cases :org.apache.hadoop.hbase.coprocessor.TestMasterCoprocessorExceptionWithAbort org.apache.hadoop.hbase.coprocessor.TestMasterCoprocessorExceptionWithRemove Enable the table priority function by default.
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-12-19 09:28:19.812855)

          Review request for hbase.

          Changes
          -------

          Simplified the design,remove the thread priority and some complex parts. Add two new test cases, one with hundreds of regions and the other use a larger row (50KB).

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriorityLargeRow.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriorityHundredRegion.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/QosRegionObserver.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterCoprocessorExceptionWithAbort.java 1220359
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterCoprocessorExceptionWithRemove.java 1220359
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/GroupTestUtil.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1220359
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1220359
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-12-19 09:28:19.812855) Review request for hbase. Changes ------- Simplified the design,remove the thread priority and some complex parts. Add two new test cases, one with hundreds of regions and the other use a larger row (50KB). Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriorityLargeRow.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriorityHundredRegion.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/QosRegionObserver.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterCoprocessorExceptionWithAbort.java 1220359 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterCoprocessorExceptionWithRemove.java 1220359 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/GroupTestUtil.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1220359 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1220359 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          Liu Jia added a comment -

          Simplified the design,remove the thread priority and some complex parts.

          Show
          Liu Jia added a comment - Simplified the design,remove the thread priority and some complex parts.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12507887/TablePriority_v17.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 24 new or modified tests.

          -1 javadoc. The javadoc tool appears to have generated -137 warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          -1 findbugs. The patch appears to introduce 86 new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.util.TestThreads
          org.apache.hadoop.hbase.ipc.TestPriorityJobQueue

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/545//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/545//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/545//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12507887/TablePriority_v17.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 24 new or modified tests. -1 javadoc. The javadoc tool appears to have generated -137 warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 86 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.util.TestThreads org.apache.hadoop.hbase.ipc.TestPriorityJobQueue Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/545//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/545//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/545//console This message is automatically generated.
          Hide
          Liu Jia added a comment -

          Reduce the time used by TestPriorityJobQueue.java.

          Show
          Liu Jia added a comment - Reduce the time used by TestPriorityJobQueue.java.
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1421/
          -----------------------------------------------------------

          (Updated 2011-12-21 02:32:29.569594)

          Review request for hbase.

          Changes
          -------

          Reduce the time used by TestPriorityJobQueue.java

          Summary
          -------

          Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities.

          This addresses bug HBase-4120.
          https://issues.apache.org/jira/browse/HBase-4120

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1220359
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1220359
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/QosRegionObserver.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterCoprocessorExceptionWithAbort.java 1220359
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterCoprocessorExceptionWithRemove.java 1220359
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/GroupTestUtil.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestActionPriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestPriorityJobQueue.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriority.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriorityHundredRegion.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriorityLargeRow.java PRE-CREATION

          Diff: https://reviews.apache.org/r/1421/diff

          Testing
          -------

          Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch
          please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client.

          Thanks,

          Jia

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1421/ ----------------------------------------------------------- (Updated 2011-12-21 02:32:29.569594) Review request for hbase. Changes ------- Reduce the time used by TestPriorityJobQueue.java Summary ------- Patch used for table priority alone,In this patch, not only tables can have different priorities but also the different actions like "get","scan","put" and "delete" can have priorities. This addresses bug HBase-4120. https://issues.apache.org/jira/browse/HBase-4120 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 1220359 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1220359 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityFunction.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityHBaseServer.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/PriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/QosRegionObserver.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterCoprocessorExceptionWithAbort.java 1220359 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterCoprocessorExceptionWithRemove.java 1220359 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/GroupTestUtil.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestActionPriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestPriorityJobQueue.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriority.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriorityHundredRegion.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/ipc/TestTablePriorityLargeRow.java PRE-CREATION Diff: https://reviews.apache.org/r/1421/diff Testing ------- Tested with test cases in TestCase_For_TablePriority_trunk_v1.patch please apply the patch of HBASE-4181 first,in some circumstances this bug will affect the performance of client. Thanks, Jia
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12508188/TablePriority_v17.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 24 new or modified tests.

          -1 javadoc. The javadoc tool appears to have generated -137 warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          -1 findbugs. The patch appears to introduce 86 new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.io.TestHeapSize

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/562//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/562//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/562//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12508188/TablePriority_v17.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 24 new or modified tests. -1 javadoc. The javadoc tool appears to have generated -137 warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 86 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.io.TestHeapSize Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/562//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/562//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/562//console This message is automatically generated.
          Hide
          Lars Hofhansl added a comment -

          Moving out of 0.94. Pull back if you feel otherwise.

          Show
          Lars Hofhansl added a comment - Moving out of 0.94. Pull back if you feel otherwise.
          Hide
          Lars Hofhansl added a comment -

          Moving out of 0.94. Pull back if you feel otherwise.

          Show
          Lars Hofhansl added a comment - Moving out of 0.94. Pull back if you feel otherwise.
          Hide
          gaojinchao added a comment -

          Hi Liu jia
          Are you working for this issue now? when do you plan to finish?

          Show
          gaojinchao added a comment - Hi Liu jia Are you working for this issue now? when do you plan to finish?
          Hide
          stack added a comment -

          Moving out of 0.96.

          Show
          stack added a comment - Moving out of 0.96.
          Hide
          Otis Gospodnetic added a comment -

          Liu Jia it looks like this got stuck last year. I quickly looked at the comments here in JIRA and see this went through some reviews and was generally welcomes. Would it be possible for you to upload a fresh patch to revive this?
          TIP: please use the same/single name for the patch. JIRA will overwrite the old one. This way devs reviewing this JIRA won't have to figure out which patch to use (yes, one can look at dates, but when there are >10 patches attached, even that becomes painful)

          Show
          Otis Gospodnetic added a comment - Liu Jia it looks like this got stuck last year. I quickly looked at the comments here in JIRA and see this went through some reviews and was generally welcomes. Would it be possible for you to upload a fresh patch to revive this? TIP: please use the same/single name for the patch. JIRA will overwrite the old one. This way devs reviewing this JIRA won't have to figure out which patch to use (yes, one can look at dates, but when there are >10 patches attached, even that becomes painful)

            People

            • Assignee:
              Liu Jia
              Reporter:
              Liu Jia
            • Votes:
              3 Vote for this issue
              Watchers:
              35 Start watching this issue

              Dates

              • Created:
                Updated:

                Time Tracking

                Estimated:
                Original Estimate - 504h
                504h
                Remaining:
                Remaining Estimate - 504h
                504h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development