OFBiz
  1. OFBiz
  2. OFBIZ-4296

JmsTopicListener started twice when distributed-cache-clear is active

    Details

      Description

      This is a follow up issue of OFBIZ-3987 and OFBIZ-4202. Apparently the problem was not properly fixed. The infinite loop during startup went away, but there still are other side effects.

      Log output excerpt:

      2011-05-26 09:43:56,336 (org.ofbiz.service.jms.JmsListenerFactory@44648ff3) [ JmsListenerFactory.java:64 :INFO ] Starting JMS Listener Factory Thread
      ...
      2011-05-26 09:44:00,499 (org.ofbiz.service.jms.JmsListenerFactory@590e6359) [ JmsListenerFactory.java:64 :INFO ] Starting JMS Listener Factory Thread
      2011-05-26 09:44:01,076 (org.ofbiz.service.jms.JmsListenerFactory@44648ff3) [   JmsTopicListener.java:88 :INFO ] Listening to topic [ofbiz-cache] on [activemq]...
      2011-05-26 09:44:01,111 (org.ofbiz.service.jms.JmsListenerFactory@590e6359) [   JmsTopicListener.java:88 :INFO ] Listening to topic [ofbiz-cache] on [activemq]...
      ...
      2011-05-26 09:44:21,081 (org.ofbiz.service.jms.JmsListenerFactory@44648ff3) [ JmsListenerFactory.java:74 :INFO ] JMS Listener Factory Thread Finished; All listeners connected.
      2011-05-26 09:44:21,111 (org.ofbiz.service.jms.JmsListenerFactory@590e6359) [ JmsListenerFactory.java:74 :INFO ] JMS Listener Factory Thread Finished; All listeners connected.
      

      What happens is this:

      DelegatorFactory.getDelegator("default") is called during startup (CatalinaContainer init). After creating the delegator and putting it in the cache it calls delegator.initEntityEcaHandler(). Somewhere inside it tries to get a dispatcher, so the first ServiceDispatcher instance is initialized. Inside the constructor code the JobManager wants to access the database and somewhere inside the EntityExpr another call to DelegatorFactory.getDelegator("default") is done. As the ECAHandler is already initialized it now initializes the DCC using EntityCacheServices.setDelegator(). This creates the second ServiceDispatcher instance and starts up the first JmsListenerFactory thread. Further up the call stack, a little later the "outer" ServiceDispatcher now also creates a JmsListenerFactory thread. So we have two listeners and every JMS message is handled twice.

      As a workaround we broke the recursion in the ServiceDispatcher constructor by doing a lazy initialization of the dispatcher in EntityCacheServices: We moved the call to EntityServiceFactory.getLocalDispatcher(delegator) out from the setDelegator method to when it is first needed.

      This doesn't seem like the proper solution, though. Maybe someone with more insight on how all the initialization of the dispatcher and delegator works can contribute a proper fix.

      1. OFBIZ-4296.patch
        5 kB
        Dimitri Unruh
      2. changeset_2683.patch
        3 kB
        Jacques Le Roux

        Issue Links

          Activity

          Hide
          Martin Kreidenweis added a comment -

          Another thing we noticed when we looked at the problem: Prior to r1090952 the DCC was initialized before the ECA handlers. Now it is the other way round. Don't know it this was intended and/or could be a problem.

          Show
          Martin Kreidenweis added a comment - Another thing we noticed when we looked at the problem: Prior to r1090952 the DCC was initialized before the ECA handlers. Now it is the other way round. Don't know it this was intended and/or could be a problem.
          Hide
          Jacques Le Roux added a comment -

          A patch using just lazy initializations (use dispatcherCache), not intended to inclusion (anyway already committed/fixed)

          Show
          Jacques Le Roux added a comment - A patch using just lazy initializations (use dispatcherCache), not intended to inclusion (anyway already committed/fixed)
          Hide
          Jacques Le Roux added a comment - - edited

          Thanks Martin,

          Your comments helped a lot. It's fixed at
          r1131144 in trunk
          r1131163 in R10.04
          r1131164 in R11.04

          I 1st tried the lazy init solution (see attached patch) but there was a little overhead.

          Show
          Jacques Le Roux added a comment - - edited Thanks Martin, Your comments helped a lot. It's fixed at r1131144 in trunk r1131163 in R10.04 r1131164 in R11.04 I 1st tried the lazy init solution (see attached patch) but there was a little overhead.
          Hide
          Dimitri Unruh added a comment -

          Hi everybody,

          I guess we should open this issue again and revert the provided patch (r1131144 in trunk).
          The solution looks really strange. The result at the moment starts just one JmsTopicListener, but only if you activated DistributedCacheClear.
          With no DistributionCacheClear JMS is not working anymore.

          So in my opinion the problem is not resolved, it is justed deferred to another place.

          Show
          Dimitri Unruh added a comment - Hi everybody, I guess we should open this issue again and revert the provided patch (r1131144 in trunk). The solution looks really strange. The result at the moment starts just one JmsTopicListener, but only if you activated DistributedCacheClear. With no DistributionCacheClear JMS is not working anymore. So in my opinion the problem is not resolved, it is justed deferred to another place.
          Hide
          Jacques Le Roux added a comment -

          Hi Dimitri,

          Reopening I'm not against, but reverting I'd not unless we have a better solution. I don't know if the doubled listeners cause problems or not, but I prefer to be safe than sorry. In the meantime you may revert "locally" if you need a listener w/out DCC, no guarantees...

          Show
          Jacques Le Roux added a comment - Hi Dimitri, Reopening I'm not against, but reverting I'd not unless we have a better solution. I don't know if the doubled listeners cause problems or not, but I prefer to be safe than sorry. In the meantime you may revert "locally" if you need a listener w/out DCC, no guarantees...
          Hide
          Dimitri Unruh added a comment -

          Jacques,

          thank you for your comment.
          I provided a new patch to resolve the listener problem.

          Anyway, I would like to do some JMS handling improvements/refactoring and will open a new ticket for this

          Show
          Dimitri Unruh added a comment - Jacques, thank you for your comment. I provided a new patch to resolve the listener problem. Anyway, I would like to do some JMS handling improvements/refactoring and will open a new ticket for this
          Hide
          Jacques Le Roux added a comment -

          Dimitri,

          In case you activate DCC your patch does not work: any JMS listener is created...

          Show
          Jacques Le Roux added a comment - Dimitri, In case you activate DCC your patch does not work: any JMS listener is created...
          Hide
          Dimitri Unruh added a comment -

          this is strange...

          I just tried it and it works fine for me

          2011-09-23 12:02:14,611 (main) [      BirtContainer.java:72 :INFO ] Start birt container
          2011-09-23 12:02:14,631 (main) [  GenericDispatcher.java:69 :INFO ] Creating new dispatcher [birt-dispatcher] (main)
          2011-09-23 12:02:14,635 (main) [      BirtContainer.java:130:INFO ] Startup birt platform
          2011-09-23 12:02:15,198 (org.ofbiz.service.jms.JmsListenerFactory@1d8f1a8) [ JmsListenerFactory.java:89 :INFO ] JMS Listener Factory Thread Finished; All listeners connected.
          2011-09-23 12:02:16,415 (main) [      BirtContainer.java:137:INFO ] Create factory object
          2011-09-23 12:02:16,462 (main) [      BirtContainer.java:143:INFO ] Create report engine
          
          Show
          Dimitri Unruh added a comment - this is strange... I just tried it and it works fine for me 2011-09-23 12:02:14,611 (main) [ BirtContainer.java:72 :INFO ] Start birt container 2011-09-23 12:02:14,631 (main) [ GenericDispatcher.java:69 :INFO ] Creating new dispatcher [birt-dispatcher] (main) 2011-09-23 12:02:14,635 (main) [ BirtContainer.java:130:INFO ] Startup birt platform 2011-09-23 12:02:15,198 (org.ofbiz.service.jms.JmsListenerFactory@1d8f1a8) [ JmsListenerFactory.java:89 :INFO ] JMS Listener Factory Thread Finished; All listeners connected. 2011-09-23 12:02:16,415 (main) [ BirtContainer.java:137:INFO ] Create factory object 2011-09-23 12:02:16,462 (main) [ BirtContainer.java:143:INFO ] Create report engine
          Hide
          Jacques Le Roux added a comment -

          Thanks Dimitri,

          Too fast: simply forgot to compile!

          Your patch is in
          trunk r1174964
          R11.04 r1174992
          R10.04 r1174990

          Show
          Jacques Le Roux added a comment - Thanks Dimitri, Too fast: simply forgot to compile! Your patch is in trunk r1174964 R11.04 r1174992 R10.04 r1174990
          Hide
          Sascha Rodekamp added a comment -

          Hm guys in my opinion the code in the JmsListenerFactory could be improved:

          Creating a Singelton in Java with the double lock is not necessarily thread safe!!
          The reason is the JVM memory model.

          I prefer to use a eager creation without synchronization overhead:

              public static JmsListenerFactory getInstance(ServiceDispatcher dispatcher) {
                  if (Holder.serviceDispatcher == null) {
                      Holder.serviceDispatcher = dispatcher;
                  }
                  return Holder.JLF;
              }
          
              private static class Holder {
                  private static ServiceDispatcher serviceDispatcher = null;
                  private static final JmsListenerFactory JLF = new JmsListenerFactory(serviceDispatcher);
              }
          

          The JmsListenerFactory is created when the Holder JLF variable is called first.

          Any opinions?
          Sascha

          Show
          Sascha Rodekamp added a comment - Hm guys in my opinion the code in the JmsListenerFactory could be improved: Creating a Singelton in Java with the double lock is not necessarily thread safe!! The reason is the JVM memory model. I prefer to use a eager creation without synchronization overhead: public static JmsListenerFactory getInstance(ServiceDispatcher dispatcher) { if (Holder.serviceDispatcher == null ) { Holder.serviceDispatcher = dispatcher; } return Holder.JLF; } private static class Holder { private static ServiceDispatcher serviceDispatcher = null ; private static final JmsListenerFactory JLF = new JmsListenerFactory(serviceDispatcher); } The JmsListenerFactory is created when the Holder JLF variable is called first. Any opinions? Sascha
          Hide
          Dimitri Unruh added a comment -

          Sascha,

          I know what you mean and I agree with you.
          But anyway, this was just a "hot-fix" for the current problem.

          As I mentioned before, I would like to do some refactoring with the hole JMS handling, because there are some more "strange" things
          So, we could do it together. What dou you think?

          Show
          Dimitri Unruh added a comment - Sascha, I know what you mean and I agree with you. But anyway, this was just a "hot-fix" for the current problem. As I mentioned before, I would like to do some refactoring with the hole JMS handling, because there are some more "strange" things So, we could do it together. What dou you think?
          Hide
          Adrian Crum added a comment -

          Another thing to look at is the bug where the JMS listener is created even when you configure OFBiz to have JMS disabled. There is no way to shut it off.

          Show
          Adrian Crum added a comment - Another thing to look at is the bug where the JMS listener is created even when you configure OFBiz to have JMS disabled. There is no way to shut it off.
          Hide
          Dimitri Unruh added a comment -

          Let's start do diskuss this things here OFBIZ-4453

          Show
          Dimitri Unruh added a comment - Let's start do diskuss this things here OFBIZ-4453

            People

            • Assignee:
              Jacques Le Roux
              Reporter:
              Martin Kreidenweis
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development