You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@helix.apache.org by dasahcc <gi...@git.apache.org> on 2017/04/04 00:27:40 UTC

[GitHub] helix pull request #82: Add Test for Batch Message ThreadPool

GitHub user dasahcc opened a pull request:

    https://github.com/apache/helix/pull/82

    Add Test for Batch Message ThreadPool

    Add Test for Batch Message ThreadPool. For easy testing, change the private type to default type.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/dasahcc/helix helix-0.6.x

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/helix/pull/82.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #82
    
----
commit cb806303f7bc5203451ed583625dbb68cc6569b8
Author: Junkai Xue <jx...@linkedin.com>
Date:   2017-04-04T00:25:33Z

    Add Test for Batch Message ThreadPool

----


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

[GitHub] helix pull request #82: Add Test for Batch Message ThreadPool

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/helix/pull/82


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

[GitHub] helix pull request #82: Add Test for Batch Message ThreadPool

Posted by dasahcc <gi...@git.apache.org>.
Github user dasahcc commented on a diff in the pull request:

    https://github.com/apache/helix/pull/82#discussion_r109812645
  
    --- Diff: helix-core/src/test/java/org/apache/helix/messaging/handling/TestResourceThreadpoolSize.java ---
    @@ -113,6 +113,35 @@ public void TestThreadPoolSizeConfig() {
         }
       }
     
    +  @Test
    +  public void testBatchMessageThreadPoolSize() throws InterruptedException {
    --- End diff --
    
    Thanks for suggestion. Just made the change for this test with the senario.


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

[GitHub] helix pull request #82: Add Test for Batch Message ThreadPool

Posted by kishoreg <gi...@git.apache.org>.
Github user kishoreg commented on a diff in the pull request:

    https://github.com/apache/helix/pull/82#discussion_r109800436
  
    --- Diff: helix-core/src/test/java/org/apache/helix/messaging/handling/TestResourceThreadpoolSize.java ---
    @@ -113,6 +113,35 @@ public void TestThreadPoolSizeConfig() {
         }
       }
     
    +  @Test
    +  public void testBatchMessageThreadPoolSize() throws InterruptedException {
    --- End diff --
    
    we need a test with number of resources with group message enabled > threapool size. This is the scenario where current code deadlocks. This test case does not cover that scenario


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

[GitHub] helix pull request #82: Add Test for Batch Message ThreadPool

Posted by kishoreg <gi...@git.apache.org>.
Github user kishoreg commented on a diff in the pull request:

    https://github.com/apache/helix/pull/82#discussion_r109814455
  
    --- Diff: helix-core/src/test/java/org/apache/helix/messaging/handling/TestResourceThreadpoolSize.java ---
    @@ -113,6 +113,35 @@ public void TestThreadPoolSizeConfig() {
         }
       }
     
    +  @Test
    +  public void testBatchMessageThreadPoolSize() throws InterruptedException {
    --- End diff --
    
    Thanks, do you see that this test case fails without my change. Might have to run it multiple times or add a delay in batch message handler before it executes the sub-messages to reproduce it. 


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

[GitHub] helix pull request #82: Add Test for Batch Message ThreadPool

Posted by dasahcc <gi...@git.apache.org>.
Github user dasahcc commented on a diff in the pull request:

    https://github.com/apache/helix/pull/82#discussion_r109972500
  
    --- Diff: helix-core/src/test/java/org/apache/helix/messaging/handling/TestResourceThreadpoolSize.java ---
    @@ -113,6 +113,35 @@ public void TestThreadPoolSizeConfig() {
         }
       }
     
    +  @Test
    +  public void testBatchMessageThreadPoolSize() throws InterruptedException {
    --- End diff --
    
    Good suggestion! I tested it manually add the delay in batch message handle, it passed! Unfortunately, we does not have proper way to have a test BatchMessageHandler. So I just add a delay for state transitions. Hope this help.


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