From: Bongjae Chang <carryel_at_korea.com>
Date: Wed, 20 May 2009 20:40:25 +0900

Hi,

Alexey wrote:
>Can you pls. make sure, that tests are working after change will be applied?

Sure.

I will file the issue and attach the patch after I will confirm all tests.

Thanks.

--
Bongjae Chang


  ----- Original Message -----
  From: Oleksiy Stashok
  To: dev_at_grizzly.dev.java.net
  Sent: Wednesday, May 20, 2009 7:06 PM
  Subject: Re: About SSLConnectionTest's failure and PipelineThreadPool issue


  Hi Bongjae,



>Probably best fix here will be switching to the default thread pool, autoconfigured by Controller.

    Right.

    Then,

    In SSLConnectionTest.java
    ---
    ...
    public static final int CLIENTS_COUNT = 10;
    ...
    public void testSeveralPackets() throws IOException {
        ...
        //controller.setThreadPool(new PipelineThreadPool("", 5, CLIENTS_COUNT * 2, Integer.MAX_VALUE, TimeUnit.MILLISECONDS));
    }
    ---

    We can fix it simply like above.

    But I found that several classes as well as SSLConnectionTest still used Controller#setThreadPool() when I searched Controller#setThreadPool()'s usages.

    Here is list.

    CustomProtocolClient.java
    CustomProtocolServer.java
    RCMTest.java
    PUBasicTest.java
    PUPreProcessorTest.java

    Do you think that above 5 classes also should be fixed?
  I think yes.


  Can you pls. make sure, that tests are working after change will be applied?


  Thank you very much!


  WBR,
  Alexey.



    Thanks.

    --
    Bongjae Chang


      ----- Original Message -----
      From: Oleksiy Stashok
      To: dev_at_grizzly.dev.java.net
      Sent: Wednesday, May 20, 2009 12:58 AM
      Subject: Re: About SSLConnectionTest's failure and PipelineThreadPool issue


      Hi Bongjae,


      I agree with your observations on PipelineThreadPool.
      Probably best fix here will be switching to the default thread pool, autoconfigured by Controller.


      What do you think?


      WBR,
      Alexey.


      On May 19, 2009, at 17:02 , Bongjae Chang wrote:


        Hi,

        I received the following error when I ran SSLConnectionTest on 4 CPUs

        -------------------------------------------------------------------------------
        Test set: com.sun.grizzly.SSLConnectionTest
        -------------------------------------------------------------------------------
        Tests run: 3, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 53.219 sec <<< FAILURE!
        testSeveralPackets(com.sun.grizzly.SSLConnectionTest) Time elapsed: 41 sec <<< FAILURE!
        junit.framework.AssertionFailedError: Handshake is not completed! testString: Hello. Client#0 Packet#000 latchCounter: 1 isConnected: false
         at junit.framework.Assert.fail(Assert.java:47)
         at junit.framework.Assert.assertTrue(Assert.java:20)
         at com.sun.grizzly.SSLConnectionTest.testSeveralPackets(SSLConnectionTest.java:187)

        When I debugged, I could find that this case was also failed for lack of thread pool's capacity. :-(

        See the following code:

        In SSLConnectionTest.java
        ---
        ...
        public static final int CLIENTS_COUNT = 10;
        ...
        public void testSeveralPackets() throws IOException {
            ...
            controller.setThreadPool(new PipelineThreadPool("", 5, CLIENTS_COUNT * 2, Integer.MAX_VALUE, TimeUnit.MILLISECONDS));
        }
        ---
          
        It means that min is 5 and max is 20.

        At first, I thought that the max which had 20 was perhaps enough for testing this case, but this case was failed.

        So, I reviewed PipelineThreadPool in order to confirm whether it increased threads until max size or not.

        See the following code:

        In PipelineThreadPool.java
        ---
        public void execute(Runnable task) {
                if (task == null){
                    throw new IllegalArgumentException("Runnable task is null");
                }

                int ac;
                while((ac=aliveworkerCount.get())<maxPoolSize && queueSize.get()>0 && running){
                    if (aliveworkerCount.compareAndSet(ac, ac+1)){
                        startWorker(new Worker(task, false));
                        return;
                    }
                }
                if (running){
                    workQueue.offer(task);
                    queueSize.incrementAndGet();
                }
            }
        ---

        Unfortunately, PipelineThreadPool allows a thread to be increased when queue size is only above 0.

        Then the sixth task will be queued without thread's increment if five former tasks are still running, and it will not be executed until the seventh task will come.

        Of course, I think that PipelineThreadPool is better than JDK's ThreadPoolExecutor in point of thread-increment-policy, but it should also be improved at this point.



        Anyway... I proposed the following patch.

        In SSLConnectionTest.java
        ---
        ...
        public static final int CLIENTS_COUNT = 10;
        ...
        public void testSeveralPackets() throws IOException {
            ...
            // Use the default thread pool which is configued automatically
            //controller.setThreadPool(new PipelineThreadPool("", 5, CLIENTS_COUNT * 2, Integer.MAX_VALUE, TimeUnit.MILLISECONDS));
        }
        ---

        Thanks.

        PS) I tested this case with my proposed thread pool(https://grizzly.dev.java.net/issues/show_bug.cgi?id=587) experimentally, it works well. :-)

        --
        Bongjae Chang