12 Replies Latest reply on Sep 16, 2006 6:38 AM by ovidiu.feodorov

    JBMESSAGING-541 - Multiple connections on a single thread wi

    clebert.suconic

      I"ve opened this thread to discuss the creationg of DurableCrashTest1 and DurableCrashTest2.


      If I have a client with two connections opened (don't know if the fact they are in the same thread matters or not), the lease timer stops and never comes back.


      I've discovered this by adding a couple of log.info into JBossRemoting during leasing and stop leasing. And basically, during addCallBAck a new client is created/closed what stops the lease timer.

      What is funny is with a single connection test, this works fine. (I mean the connection is detected without any problem)


      I have also opened a JIRA on JBossRemoting:

      http://jira.jboss.org/jira/browse/JBREM-596


      public void testSimplestDurableSubscription() throws Exception
       {
       ConnectionFactory cf = (ConnectionFactory)ic.lookup("ConnectionFactory");
       Topic topic = (Topic)ic.lookup("/topic/TopicCrash");
      
       Connection conn = cf.createConnection();
       conn.setClientID("client1");
       conn.start();
      
       Connection conn2 = cf.createConnection(); // funny... if I remove everything related to conn2, the lease thread works
       conn2.setClientID("client2");
       conn2.start();
      
       Session s = conn.createSession(true, Session.AUTO_ACKNOWLEDGE);
       Session s2 = conn2.createSession(true, Session.AUTO_ACKNOWLEDGE);
      
       MessageProducer prod = s.createProducer(topic);
       prod.setDeliveryMode(DeliveryMode.PERSISTENT);
      
       MessageConsumer durable = s.createDurableSubscriber(topic, "subs1");
       MessageConsumer durable2 = s2.createDurableSubscriber(topic, "subs2");
      
       //conn2.close(); -- do not remote this comment. It was meant to not close the connection
      
      
       for (int i=0;i<10;i++)
       {
       prod.send(s.createTextMessage("k"+i));
       }
       s.commit();
      
       conn.close();
       conn = cf.createConnection();
       conn.setClientID("client1");
       conn.start();
       s = conn.createSession(true, Session.AUTO_ACKNOWLEDGE);
       durable = s.createDurableSubscriber(topic, "subs1");
      
       for (int i=0;i<2;i++)
       {
       TextMessage tm = (TextMessage)durable.receive(1000);
       assertNotNull(tm);
       s.commit();
       System.out.println(tm.getText());
       assertEquals("k" + i, tm.getText());
       }
      
       //conn.close();-- do not remote this comment. It was meant to not close the connection
      
       //System.exit(0); -- this is not needed as there is not tearDown, the client VM will simply be finished the same way an exit would do, and this is better since it will keep proper JUNIT report outputs
       }
      


        • 1. Re: JBMESSAGING-541 - Multiple connections on a single threa
          ovidiu.feodorov

          To limit the scope of the discussion:

          The problem we're talking about has nothing to do with durable subscriptions. DurableCrashTest1 and DurableCrashTest2, while semantically correct, were designed to investigate a completely different bug, and Clebert stumbled over the current one (http://jira.jboss.org/jira/browse/JBREM-596), just by chance.

          Let's create a Messaging test that exhibits the problem using the minimal possible number of moving parts.

          Clebert is working on that. I would suggest "TwoConnectionsCrashTest" as name of the test.

          Let's post the conclusions on this thread, and not in private e-mails.

          • 2. Re: JBMESSAGING-541 - Multiple connections on a single threa
            clebert.suconic

            When I was doing these tests, I got side tracked by another problem.

            I changed SimpleConnectionManager to add debug System.out on my local copy, and because of some AOP recompilation, the ConnectionListener was not being set any more.


            After doing a complete clean, it looks like the problem is when having two connections on the client (there is only one client per VM) the second connection didn't get it's delivery list cleared.

            I'm doing some investigation on this, but it doesn't look like a remoting bug.

            • 3. Re: JBMESSAGING-541 - Multiple connections on a single threa
              clebert.suconic

              Today I cut the testcase until the minimal point of the failure.

              Basically if you close the first connection (Where Client was created) Lease would stop working.

              A testcase that would cause Lease to stop would be:

              conn1 = cf.createConnection();
              conn1.setClientID("test1");
              conn1.start();
              
              conn2 = cf.createConnection();
              conn2.setClientID("test2");
              conn2.start();
              
              conn1.close();



              I added ClientCrashTest::testClientCrashWithTwoConnections which is calling CreateTwoClientOnServerCommand on the server side.


              The test consistenly fails.


              Clebert Suconic



              • 4. Re: JBMESSAGING-541 - Multiple connections on a single threa
                clebert.suconic

                Found the root cause of this.

                Remoting will always reuse the same internal client to a given locator.

                On InvokerRegistry.createClientInvoker if a previous locator was used with the same properties, the first locator will be used.

                On remoting there is a testcase testing this under org.jboss.test.remoting.lease.socket.multiple. If you change both connections to use the same id you will have the behavior described on this bug.


                so, to fix this we could either be doing the proper reference counting before stopping the lease, or having each connection setting a different nameId.

                Having each client Connection having a property differntiating locator will force each connection to open a new socket (which is what we wanted anyways I believe).

                • 5. Re: JBMESSAGING-541 - Multiple connections on a single threa
                  clebert.suconic

                  Just confirmed:

                  If changed JMSRemotingConnection::start like this


                  
                   public static int count = 1;
                  
                   ....
                  
                  
                   public void start() throws Throwable
                   {
                   // Enable client pinging. Server leasing is enabled separately on the server side
                  
                   Map config = new HashMap();
                   config.put(Client.ENABLE_LEASE, String.valueOf(clientPing));
                   config.put("clientName", "connection"+(count++));
                  
                   .......
                  



                  The leasing always works

                  • 6. Re: JBMESSAGING-541 - Multiple connections on a single threa
                    ovidiu.feodorov

                    At first, I was tempted to say that since a JMS Connection is a heavy-weight object anyway, it does not matter too much if each JMS Connection instance creates and maintains its own separate TCP/IP connection (which it will do if we explicitly and distinctly "name" remoting clients, as you suggested).

                    However, the physical TCP/IP connection pooling offered by remoting may prove beneficial in some use cases. Imagine 100 co-located JMS clients opening connections to a server, but sending messages at a relatively low rate. If we disable remoting TCP/IP connection pooling, we'll have 100 distinct physical TCP/IP connection, each of them firing from time to time, and sitting idle the rest of the time. If we use TCP/IP connection pooling, it is very likely to actually start a very low number of physical TCP/IP connections, the actual number depending on the send rate, as each TCP/IP connection is allocated on an invocation basis.

                    I would rather fix Remoting.

                    I would suspect fixing it is as simple as NOT disabling lease pinging unless ClientInvokerHolder.counter for the client being disconnected is 1. Any value higher than 1 means that the client is still in use by somebody else. However, this is just a superficial attempt to come with a solution and needs to be validated by the Remoting team.

                    As soon as the Remoting team produces a fix, we can release CR5 based on a remoting snapshot.

                    • 7. Re: JBMESSAGING-541 - Multiple connections on a single threa
                      timfox

                       

                      "ovidiu.feodorov@jboss.com" wrote:

                      I would rather fix Remoting.

                      I would suspect fixing it is as simple as NOT disabling lease pinging unless ClientInvokerHolder.counter for the client being disconnected is 1. Any value higher than 1 means that the client is still in use by somebody else. However, this is just a superficial attempt to come with a solution and needs to be validated by the Remoting team.

                      As soon as the Remoting team produces a fix, we can release CR5 based on a remoting snapshot.


                      I agree which should do a quick fix for now.

                      All this is moot over the next few months, since remoting is incompatible with an AMQP style model, which we're going to have to implement if we're going to support AMQP.

                      So we're probably going to have to throw the socket transport away anyway.

                      • 8. Re: JBMESSAGING-541 - Multiple connections on a single threa
                        ovidiu.feodorov

                         

                        Tim wrote:
                        So we're probably going to have to throw the socket transport away anyway.


                        Maybe.

                        However, the fix for the current issue should be trivial, so I see no reason not to do it.

                        It think it would be a good tactical decision NOT to attack the AMQP/Remoting issue now, but wait until after the 1.2 Alpha release.

                        • 9. Re: JBMESSAGING-541 - Multiple connections on a single threa
                          timfox

                           

                          "ovidiu.feodorov@jboss.com" wrote:

                          However, the fix for the current issue should be trivial, so I see no reason not to do it.


                          That's why I said we should do a quick fix for now in my previous post. :)


                          It think it would be a good tactical decision NOT to attack the AMQP/Remoting issue now, but wait until after the 1.2 Alpha release.


                          I'm not suggesting we do this now, just making the point we're going to have do this over the next few months. So let's not waste too much time on remoting related issues (this includes the http or multiplex transports)

                          • 10. Re: JBMESSAGING-541 - Multiple connections on a single threa
                            ovidiu.feodorov

                             

                            Tim wrote:
                            So let's not waste too much time on remoting related issues (this includes the http or multiplex transports)


                            HTTP is Ron's entry point in Messaging, he's already advanced in getting it working, and one of the possible approaches to the Remoting issue is to extend it with asynch support, but still use things as HTTP as they are.

                            Multiplex is a different issue, it will definitely NOT be Ron's next task after he is done with HTTP.


                            • 11. Re: JBMESSAGING-541 - Multiple connections on a single threa
                              timfox

                               

                              "ovidiu.feodorov@jboss.com" wrote:


                              HTTP is Ron's entry point in Messaging



                              So yes there is value in doing this from the point of view of getting acquainted with the code base.

                              And, well... yes maybe we can keep the old http transport, but personally I don't think this is the way to go. I am sure we will debate this a lot in the future weeks/months :)

                              I am all for a clean new approach where we don't have to mess around with callback servers and other unnecessary baggage.

                              My POV would be, until we have come up with a definite plan w.r.t. remoting and how we're going to implement each transport then we shouldn't waste time working on something which has a good chance of being chucked.


                              Just my 2c

                              • 12. Re: JBMESSAGING-541 - Multiple connections on a single threa
                                ovidiu.feodorov

                                There's a lot to talk about Remoting, but how about leaving this thread alone, it was orginally created for a very specific problem and now we're diverging. As far as I am concerned, we can use http://www.jboss.com/index.html?module=bb&op=viewtopic&p=3971508 for that.