Camel
  1. Camel
  2. CAMEL-4345

Synchronized code causes long delays and hangs for big applications especially with Blueprint

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.8.0
    • Fix Version/s: 2.8.2, 2.9.0
    • Component/s: camel-core
    • Labels:
      None
    • Environment:

      Linux and Mac multicore machines

    • Patch Info:
      Patch Available
    • Estimated Complexity:
      Unknown

      Description

      The DefaultCamelContext uses synchronized "endpoints" which ends up ultimately extending a LinkedHashMap through the LRUCache. The LinkedHashMap is obviously not thread safe, so it requires synchronized guards when accessing the endpoints object. This especially happens in the getEndpoint(s) calls in the DefaultCamelContext. In large systems with lots of routes and on multicore systems, dynamically created routes (and many routes) can cause long delays and hang for long times since route creation and the starting of the camel route can occur in unison with synchronization. In a blueprint container, such as Karaf, this can cause timeouts on the bundle and camel routes will appear to hang indefinately. Thread dumps show the hangs occur on the synchronized call in getEndpoint(s). The fix for this is to use concurrent apis as much as possible and remove the synchronized code. I refactored the LRUCache/LRUSoftCache to use Google's ConcurrentLinkedHashMap (ASL2 License http://code.google.com/p/concurrentlinkedhashmap) and removed the synchronized code that locks the endpoints object. This should remove the hangs since the locks are no longer required. Since COncurrentLinkedHashmap is not OSGi ready, I have shaded the classes in core. On my executions, all unit tests pass with this refactoring using the concurrent code. This should speed up Camel on multicore systems that have lots of routes.

      1. CAMEL-4345.patch
        15 kB
        Jeff Genender

        Activity

        Hide
        Jeff Genender added a comment -

        Patch attached against TRUNK as of r1158987

        Show
        Jeff Genender added a comment - Patch attached against TRUNK as of r1158987
        Hide
        Claus Ibsen added a comment -

        Nice patch.

        By shading it into camel-core I like the fact that the camel-core dependency don't change.
        However are there people who would not like shading this into camel-core?

        And are there any Apache policy that would discourage this fact?
        eg the JAR contains .class files which are not from Apache ?

        If just the JDK had this class already. Even JDK7 dont have it, right?

        Show
        Claus Ibsen added a comment - Nice patch. By shading it into camel-core I like the fact that the camel-core dependency don't change. However are there people who would not like shading this into camel-core? And are there any Apache policy that would discourage this fact? eg the JAR contains .class files which are not from Apache ? If just the JDK had this class already. Even JDK7 dont have it, right?
        Hide
        Edstrom Johan added a comment -

        At least the google code is ASF License, would that help some?

        Show
        Edstrom Johan added a comment - At least the google code is ASF License, would that help some?
        Hide
        Claus Ibsen added a comment -

        Yeah the ASL license sure does

        I think back in the days Spring framework included the aop alliance JAR in the Spring JAR directly as well.

        Show
        Claus Ibsen added a comment - Yeah the ASL license sure does I think back in the days Spring framework included the aop alliance JAR in the Spring JAR directly as well.
        Hide
        Daniel Kulp added a comment -

        I have no problem shading it in, but does it need to be exported from the bundle in OSGi?

        Since it's completely an internal field, I don't think it does. I'd definitely prefer it didn't get exported so if some other project has a bundle with it, we don't conflict and such.

        Show
        Daniel Kulp added a comment - I have no problem shading it in, but does it need to be exported from the bundle in OSGi? Since it's completely an internal field, I don't think it does. I'd definitely prefer it didn't get exported so if some other project has a bundle with it, we don't conflict and such.
        Hide
        Ben Manes added a comment -

        Please open an issue to OSGi-ify CLHM and I'll do it for the next release (no timeframe).

        As Google Guava is OSGi ready and I helped port the algorithm into MapMaker, that may also be a reasonable choice if it becomes a concern.

        I'll be meeting up with Doug Lea in Sept. to discuss caching for JDK8, so maybe we'll see something in the standard libs eventually.

        Cheers,
        Ben
        (CLHM author, Googler)

        P.S. Don't you love Google Alerts?

        Show
        Ben Manes added a comment - Please open an issue to OSGi-ify CLHM and I'll do it for the next release (no timeframe). As Google Guava is OSGi ready and I helped port the algorithm into MapMaker, that may also be a reasonable choice if it becomes a concern. I'll be meeting up with Doug Lea in Sept. to discuss caching for JDK8, so maybe we'll see something in the standard libs eventually. Cheers, Ben (CLHM author, Googler) P.S. Don't you love Google Alerts?
        Hide
        Hadrian Zbarcea added a comment -

        @Ben, thanks for the offer and welcome to the Camel community .

        Show
        Hadrian Zbarcea added a comment - @Ben, thanks for the offer and welcome to the Camel community .
        Hide
        Hadrian Zbarcea added a comment -

        Patch applied in r1159457 with thanks to Jeff. Jeff, we expect more from you .
        There are a few checkstyle issues in the patch I will correct in the next hour or so and mark this issue as resolved.

        Show
        Hadrian Zbarcea added a comment - Patch applied in r1159457 with thanks to Jeff. Jeff, we expect more from you . There are a few checkstyle issues in the patch I will correct in the next hour or so and mark this issue as resolved.
        Hide
        Hadrian Zbarcea added a comment -

        Fixed checkstyle and also removed the OSGi export of the google stuff. The intent I believe was to make it accessible to components, but we can do that via a utility factory if/when needed.

        Thanks Jeff!

        Show
        Hadrian Zbarcea added a comment - Fixed checkstyle and also removed the OSGi export of the google stuff. The intent I believe was to make it accessible to components, but we can do that via a utility factory if/when needed. Thanks Jeff!
        Hide
        Claus Ibsen added a comment -

        The osgi manifest contains import of the google code

        Import-Package: com.googlecode.concurrentlinkedhashmap,javax.activatio
         n;resolution:=optional,javax.management,javax.management.modelmbean,j
         avax.management.remote,javax.naming,javax.naming.spi,javax.net.ssl,ja
         vax.xml.bind;resolution:=optional,javax.xml.bind.annotation;resolutio
         n:=optional,javax.xml.bind.annotation.adapters;resolution:=optional,j
         avax.xml.namespace,javax.xml.parsers,javax.xml.stream;resolution:=opt
         ional,javax.xml.transform,javax.xml.transform.dom,javax.xml.transform
         .sax,javax.xml.transform.stream,javax.xml.validation,javax.xml.xpath,
         org.apache.xalan.xsltc.trax;resolution:=optional,org.osgi.framework;v
         ersion="[1.4,2)",org.slf4j;version="[1.6,2)",org.w3c.dom,org.xml.sax
        

        I cannot see that its being exported, so I would assume we are almost okay.

        What if another bundle exports com.googlecode.concurrentlinkedhashmap,
        then camel-core could potential pickup that instead from itself, or am I mistaking?

        Show
        Claus Ibsen added a comment - The osgi manifest contains import of the google code Import-Package: com.googlecode.concurrentlinkedhashmap,javax.activatio n;resolution:=optional,javax.management,javax.management.modelmbean,j avax.management.remote,javax.naming,javax.naming.spi,javax.net.ssl,ja vax.xml.bind;resolution:=optional,javax.xml.bind.annotation;resolutio n:=optional,javax.xml.bind.annotation.adapters;resolution:=optional,j avax.xml.namespace,javax.xml.parsers,javax.xml.stream;resolution:=opt ional,javax.xml.transform,javax.xml.transform.dom,javax.xml.transform .sax,javax.xml.transform.stream,javax.xml.validation,javax.xml.xpath, org.apache.xalan.xsltc.trax;resolution:=optional,org.osgi.framework;v ersion= "[1.4,2)" ,org.slf4j;version= "[1.6,2)" ,org.w3c.dom,org.xml.sax I cannot see that its being exported, so I would assume we are almost okay. What if another bundle exports com.googlecode.concurrentlinkedhashmap, then camel-core could potential pickup that instead from itself, or am I mistaking?
        Hide
        Claus Ibsen added a comment -

        Did anyone create a ticket at Google CLHM to ask for osgify the distribution, as Ben said we should do?

        Show
        Claus Ibsen added a comment - Did anyone create a ticket at Google CLHM to ask for osgify the distribution, as Ben said we should do?
        Hide
        Jeff Genender added a comment -

        Claus, yes I opened an issue to OSGI-ify ConcurrentLinkedHashMap.

        Good catch on the import... it should not be in there.... clearly the Felix runs before the shade

        The fix is in the camel-core pom, right under the <camel.osgi.import>, add this:

        !com.googlecode.concurrentlinkedhashmap,
        

        That will fix it.

        Show
        Jeff Genender added a comment - Claus, yes I opened an issue to OSGI-ify ConcurrentLinkedHashMap. Good catch on the import... it should not be in there.... clearly the Felix runs before the shade The fix is in the camel-core pom, right under the <camel.osgi.import>, add this: !com.googlecode.concurrentlinkedhashmap, That will fix it.
        Hide
        Claus Ibsen added a comment -

        I have applied that last change in osgi imports. And ran the osgi tests. All seems fine. Even camel-cxf which uses LRUCache from camel-core. It works also in osgi with that change above.

        Show
        Claus Ibsen added a comment - I have applied that last change in osgi imports. And ran the osgi tests. All seems fine. Even camel-cxf which uses LRUCache from camel-core. It works also in osgi with that change above.

          People

          • Assignee:
            Hadrian Zbarcea
            Reporter:
            Jeff Genender
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development