Solr
  1. Solr
  2. SOLR-8180

Missing logging dependency in solrj-lib for SolrJ

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 6.0
    • Fix Version/s: 5.4
    • Component/s: SolrJ
    • Labels:
      None

      Description

      When using DBVisualizer, SquirrelSQL, or Java JDBC with the Solr JDBC driver, an additional logging dependency must be added otherwise the following exception occurs:

      org.apache.solr.common.SolrException: Unable to create HttpClient instance. 
      	at org.apache.solr.client.solrj.impl.HttpClientUtil$HttpClientFactory.createHttpClient(HttpClientUtil.java:393)
      	at org.apache.solr.client.solrj.impl.HttpClientUtil.createClient(HttpClientUtil.java:124)
      	at org.apache.solr.client.solrj.impl.CloudSolrClient.<init>(CloudSolrClient.java:196)
      	at org.apache.solr.client.solrj.io.SolrClientCache.getCloudSolrClient(SolrClientCache.java:47)
      	at org.apache.solr.client.solrj.io.sql.ConnectionImpl.<init>(ConnectionImpl.java:51)
      	at org.apache.solr.client.solrj.io.sql.DriverImpl.connect(DriverImpl.java:108)
      	at org.apache.solr.client.solrj.io.sql.DriverImpl.connect(DriverImpl.java:76)
      	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
      	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
      	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
      	at java.lang.reflect.Method.invoke(Method.java:497)
      	at com.onseven.dbvis.h.B.D.ᅣチ(Z:1548)
      	at com.onseven.dbvis.h.B.F$A.call(Z:1369)
      	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
      	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
      	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
      	at java.lang.Thread.run(Thread.java:745)
      Caused by: java.lang.reflect.InvocationTargetException
      	at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
      	at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
      	at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
      	at java.lang.reflect.Constructor.newInstance(Constructor.java:422)
      	at org.apache.solr.client.solrj.impl.HttpClientUtil$HttpClientFactory.createHttpClient(HttpClientUtil.java:391)
      	... 16 more
      Caused by: java.lang.NoClassDefFoundError: org/apache/commons/logging/LogFactory
      	at org.apache.http.impl.client.CloseableHttpClient.<init>(CloseableHttpClient.java:58)
      	at org.apache.http.impl.client.AbstractHttpClient.<init>(AbstractHttpClient.java:287)
      	at org.apache.http.impl.client.DefaultHttpClient.<init>(DefaultHttpClient.java:128)
      	at org.apache.http.impl.client.SystemDefaultHttpClient.<init>(SystemDefaultHttpClient.java:116)
      	... 21 more
      
      1. SOLR_8180_jcl_over_slf4j.patch
        18 kB
        David Smiley
      2. SOLR_8180_jcl_over_slf4j.patch
        19 kB
        David Smiley
      3. SOLR-8180.patch
        1 kB
        Kevin Risden

        Issue Links

          Activity

          Hide
          Erick Erickson added a comment -

          Joel Bernstein just making sure you see this, another patch by Kevin

          Show
          Erick Erickson added a comment - Joel Bernstein just making sure you see this, another patch by Kevin
          Hide
          Joel Bernstein added a comment - - edited

          Yeah, I saw this earlier. But it confused me a little. This is a Solrj dependency that for some reason does not come as part of the solrj-lib. I was wondering if there is a dependency conflict. Chris Hostetter (Unused), wondering if you knew the background on the logging dependancies?

          Show
          Joel Bernstein added a comment - - edited Yeah, I saw this earlier. But it confused me a little. This is a Solrj dependency that for some reason does not come as part of the solrj-lib. I was wondering if there is a dependency conflict. Chris Hostetter (Unused) , wondering if you knew the background on the logging dependancies?
          Hide
          Shawn Heisey added a comment -

          I believe that the primary resource requiring commons-logging is HttpClient. I haven't checked to see if it is required by any other dependencies.

          The commons-logging dependency will normally be satisfied by the jars pulled in for logging – those other than slf4j-api, which is included in the .war file for Solr and solrj-lib for SolrJ. For the Solr server, this is satisfied by jcl-over-slf4j-1.7.7.jar ... which you can find in the Solr download at server/lib/ext, with the other logging jars. That jar captures calls to commons-logging and directs them through slf4j, which then sends them to whatever logging framework is bound.

          Since 4.3, the bundled example server has used log4j 1.2 as the binding. Versions prior to 4.3 used java.util.logging, and the logging jars were bundled in the .war file.

          If the user wants to actually use commons-logging for their slf4j binding, then they would not use jcl-over-slf4j-1.7.7.jar ... they would use the actual commons-logging jar and its dependencies. An additional jar would be required to redirect log4j through slf4j.

          Some generic info about slf4j logging and Solr:

          https://wiki.apache.org/solr/SolrLogging

          Just like the Solr server, SolrJ requires additional logging jars not included in solrj-lib. The reason they are not included there is because we do not know what kind of logging the user wants. They are free to choose whatever meets their needs. This probably needs a mention in the docs.

          Show
          Shawn Heisey added a comment - I believe that the primary resource requiring commons-logging is HttpClient. I haven't checked to see if it is required by any other dependencies. The commons-logging dependency will normally be satisfied by the jars pulled in for logging – those other than slf4j-api, which is included in the .war file for Solr and solrj-lib for SolrJ. For the Solr server , this is satisfied by jcl-over-slf4j-1.7.7.jar ... which you can find in the Solr download at server/lib/ext, with the other logging jars. That jar captures calls to commons-logging and directs them through slf4j, which then sends them to whatever logging framework is bound. Since 4.3, the bundled example server has used log4j 1.2 as the binding. Versions prior to 4.3 used java.util.logging, and the logging jars were bundled in the .war file. If the user wants to actually use commons-logging for their slf4j binding, then they would not use jcl-over-slf4j-1.7.7.jar ... they would use the actual commons-logging jar and its dependencies. An additional jar would be required to redirect log4j through slf4j. Some generic info about slf4j logging and Solr: https://wiki.apache.org/solr/SolrLogging Just like the Solr server, SolrJ requires additional logging jars not included in solrj-lib. The reason they are not included there is because we do not know what kind of logging the user wants. They are free to choose whatever meets their needs. This probably needs a mention in the docs.
          Hide
          Shawn Heisey added a comment -

          I updated the wiki page that I linked so it mentions SolrJ and logging jars. The reference guide may need editing as well.

          Show
          Shawn Heisey added a comment - I updated the wiki page that I linked so it mentions SolrJ and logging jars. The reference guide may need editing as well.
          Hide
          Kevin Risden added a comment -

          I understand that configuring logging can be specific to ones application. However using the JDBC part SolrJ out of the box with something like DBVisualizer/SquirrelSQL shouldn't require finding more log jars to start.

          It looks like commons-logging can be replaced with jcl-over-slf4j and then everything out of the box would go through slf4j. It currently looks like with SolrJ that part of the logging goes through commons-logging from the httpclient and the rest is through slf4j.

          Would it make sense to have jcl-over-slf4j included by default in solrj-lib instead of trying to include commons-logging? This seems like it would match closer to what Solr server does out of the box.

          Show
          Kevin Risden added a comment - I understand that configuring logging can be specific to ones application. However using the JDBC part SolrJ out of the box with something like DBVisualizer/SquirrelSQL shouldn't require finding more log jars to start. It looks like commons-logging can be replaced with jcl-over-slf4j and then everything out of the box would go through slf4j. It currently looks like with SolrJ that part of the logging goes through commons-logging from the httpclient and the rest is through slf4j. Would it make sense to have jcl-over-slf4j included by default in solrj-lib instead of trying to include commons-logging? This seems like it would match closer to what Solr server does out of the box.
          Hide
          David Smiley added a comment -

          I agree with you Kevin. Throughout Solr's releases, my SolrJ using search apps have had to monkey with its logging dependencies due to either missing log dependencies, or to correct erroneous dependencies. +1 to jcl-over-slf4j. What do you think Steve Rowe?

          Show
          David Smiley added a comment - I agree with you Kevin. Throughout Solr's releases, my SolrJ using search apps have had to monkey with its logging dependencies due to either missing log dependencies, or to correct erroneous dependencies. +1 to jcl-over-slf4j. What do you think Steve Rowe ?
          Hide
          Steve Rowe added a comment -

          +1 to jcl-over-slf4j.

          To be clear, we're talking about adding this in two places, right?:

          1. dist/solrj-lib/ in the binary distribution
          2. As a non-optional dependency in the solrj POM

          +1 to the above

          Show
          Steve Rowe added a comment - +1 to jcl-over-slf4j. To be clear, we're talking about adding this in two places, right?: dist/solrj-lib/ in the binary distribution As a non-optional dependency in the solrj POM +1 to the above
          Hide
          David Smiley added a comment -

          Yes; +1 to that. I'll keep this on my backlog until I have time to if someone doesn't get it done first. Should be simple.

          Show
          David Smiley added a comment - Yes; +1 to that. I'll keep this on my backlog until I have time to if someone doesn't get it done first. Should be simple.
          Hide
          Kevin Risden added a comment -

          Updated title and description based on the comments.

          Show
          Kevin Risden added a comment - Updated title and description based on the comments.
          Hide
          Kevin Risden added a comment -

          in solr/solrj/ivy.xml there are two slf4j dependencies tagged for test only.

              <dependency org="org.slf4j" name="jcl-over-slf4j" rev="${/org.slf4j/jcl-over-slf4j}" conf="test"/>
              <dependency org="org.slf4j" name="slf4j-log4j12" rev="${/org.slf4j/slf4j-log4j12}" conf="test"/>
          

          Should they both be included in solrj-lib or only jcl-over-slf4j based on what was discussed above? It would be changing the conf to compile.

          I'm not sure what issues would come up if slf4j-log4j12 were to be included. It would make the default logging be log4j but at least there would be a concrete implementation.

          I'm not sure if changing ivy.xml would also change the SolrJ POM.

          Show
          Kevin Risden added a comment - in solr/solrj/ivy.xml there are two slf4j dependencies tagged for test only. <dependency org= "org.slf4j" name= "jcl-over-slf4j" rev= "${/org.slf4j/jcl-over-slf4j}" conf= "test" /> <dependency org= "org.slf4j" name= "slf4j-log4j12" rev= "${/org.slf4j/slf4j-log4j12}" conf= "test" /> Should they both be included in solrj-lib or only jcl-over-slf4j based on what was discussed above? It would be changing the conf to compile. I'm not sure what issues would come up if slf4j-log4j12 were to be included. It would make the default logging be log4j but at least there would be a concrete implementation. I'm not sure if changing ivy.xml would also change the SolrJ POM.
          Hide
          Shawn Heisey added a comment -

          If those jars are included, jul-to-slf4j and the true log4j jar are also very likely required.

          Show
          Shawn Heisey added a comment - If those jars are included, jul-to-slf4j and the true log4j jar are also very likely required.
          Hide
          Steve Rowe added a comment -

          I'm not sure if changing ivy.xml would also change the SolrJ POM.

          It's supposed to automagically, but there has been special handling in the past around logging jars, so I'm not sure it'll work without undoing that if it's still there.

          Show
          Steve Rowe added a comment - I'm not sure if changing ivy.xml would also change the SolrJ POM. It's supposed to automagically, but there has been special handling in the past around logging jars, so I'm not sure it'll work without undoing that if it's still there.
          Hide
          Kevin Risden added a comment -

          If there are no test dependencies in solr/solrj/ivy.xml I receive the following error stating that "solr/solrj/test-lib does not exist" when running "ant test" in solr/solrj:

          BUILD FAILED
          /home/travis/build/risdenk/lucene-solr/solr/build.xml:246: The following error occurred while executing this line:
          /home/travis/build/risdenk/lucene-solr/solr/common-build.xml:516: The following error occurred while executing this line:
          /home/travis/build/risdenk/lucene-solr/lucene/common-build.xml:796: The following error occurred while executing this line:
          /home/travis/build/risdenk/lucene-solr/lucene/common-build.xml:810: The following error occurred while executing this line:
          /home/travis/build/risdenk/lucene-solr/lucene/common-build.xml:1944: /home/travis/build/risdenk/lucene-solr/solr/solrj/test-lib does not exist.
          

          If I duplicate the two slf4j test dependencies and add them also as compile then the "ant test" works.

          Show
          Kevin Risden added a comment - If there are no test dependencies in solr/solrj/ivy.xml I receive the following error stating that "solr/solrj/test-lib does not exist" when running "ant test" in solr/solrj: BUILD FAILED /home/travis/build/risdenk/lucene-solr/solr/build.xml:246: The following error occurred while executing this line: /home/travis/build/risdenk/lucene-solr/solr/common-build.xml:516: The following error occurred while executing this line: /home/travis/build/risdenk/lucene-solr/lucene/common-build.xml:796: The following error occurred while executing this line: /home/travis/build/risdenk/lucene-solr/lucene/common-build.xml:810: The following error occurred while executing this line: /home/travis/build/risdenk/lucene-solr/lucene/common-build.xml:1944: /home/travis/build/risdenk/lucene-solr/solr/solrj/test-lib does not exist. If I duplicate the two slf4j test dependencies and add them also as compile then the "ant test" works.
          Hide
          David Smiley added a comment -

          Here's a patch.
          Most contribs had only one test dependency, this one. Now that it's gone, these test-lib directory can go away and some extra lines in these build.xml's were removed. I decided to leave the "test" configuration declaration line in these ivy.xml's.

          I'm having an issue with my maven build right now before it gets to some of these contribs; not likely related to this patch. My ant build is running now. I'd appreciate someone else running the maven build too.

          On an unrelated note, I noticed the Maven build output complained on each test that, essentially, -XX:MaxPermSize=256M doesn't do anything in Java 8. I'd like to do a lone commit on trunk only to remove that from the parent pom.xml.template.

          Show
          David Smiley added a comment - Here's a patch. Most contribs had only one test dependency, this one. Now that it's gone, these test-lib directory can go away and some extra lines in these build.xml's were removed. I decided to leave the "test" configuration declaration line in these ivy.xml's. I'm having an issue with my maven build right now before it gets to some of these contribs; not likely related to this patch. My ant build is running now. I'd appreciate someone else running the maven build too. On an unrelated note, I noticed the Maven build output complained on each test that, essentially, -XX:MaxPermSize=256M doesn't do anything in Java 8. I'd like to do a lone commit on trunk only to remove that from the parent pom.xml.template.
          Hide
          David Smiley added a comment -

          Updating patch to revert me deleting too much in dataimporthandler-extras. The ant build passes.

          On my machine, the maven build fails on the TestRecoveryHdfs test for some reason or another (not investigated yet). Also the morfologik one does but I fixed that locally due to what appears to be a pom that needs to be updated – reported to LUCENE-6833 already.

          Show
          David Smiley added a comment - Updating patch to revert me deleting too much in dataimporthandler-extras. The ant build passes. On my machine, the maven build fails on the TestRecoveryHdfs test for some reason or another (not investigated yet). Also the morfologik one does but I fixed that locally due to what appears to be a pom that needs to be updated – reported to LUCENE-6833 already.
          Hide
          Uwe Schindler added a comment -

          On an unrelated note, I noticed the Maven build output complained on each test that, essentially, -XX:MaxPermSize=256M doesn't do anything in Java 8. I'd like to do a lone commit on trunk only to remove that from the parent pom.xml.template.

          Yes, this is not needed on Java 8, so we should remove it in trunk. On branch_5x, Ant build is a bit better, because it detects Java version and only enables permgen on Java 7. Not sure if this works easily with Maven (maybe profiles help)...

          Show
          Uwe Schindler added a comment - On an unrelated note, I noticed the Maven build output complained on each test that, essentially, -XX:MaxPermSize=256M doesn't do anything in Java 8. I'd like to do a lone commit on trunk only to remove that from the parent pom.xml.template. Yes, this is not needed on Java 8, so we should remove it in trunk. On branch_5x, Ant build is a bit better, because it detects Java version and only enables permgen on Java 7. Not sure if this works easily with Maven (maybe profiles help)...
          Hide
          Uwe Schindler added a comment -

          On my machine, the maven build fails on the TestRecoveryHdfs test for some reason or another (not investigated yet).

          Known problem. This is one reason why the ASF Jenkins Maven builds pass -DskipTests to the Maven build. There is an issue open, don't know issue number... It also applies to the mapreduce/morphlines contribs, also fail only in Maven but not Ant. I have the feeling this is some surefire incompatibility.

          Show
          Uwe Schindler added a comment - On my machine, the maven build fails on the TestRecoveryHdfs test for some reason or another (not investigated yet). Known problem. This is one reason why the ASF Jenkins Maven builds pass -DskipTests to the Maven build. There is an issue open, don't know issue number... It also applies to the mapreduce/morphlines contribs, also fail only in Maven but not Ant. I have the feeling this is some surefire incompatibility.
          Hide
          David Smiley added a comment -

          Thanks for your input Uwe. So this explains why the maven build always seems to pass on Jenkins. Perhaps we should have some sort of annotation for a test to flag that it can should be skipped during a maven build.

          Any way, I plan to commit this patch tomorrow if I don't get any further feedback to the contrary.

          Show
          David Smiley added a comment - Thanks for your input Uwe. So this explains why the maven build always seems to pass on Jenkins. Perhaps we should have some sort of annotation for a test to flag that it can should be skipped during a maven build. Any way, I plan to commit this patch tomorrow if I don't get any further feedback to the contrary.
          Hide
          ASF subversion and git services added a comment -

          Commit 1717481 from David Smiley in branch 'dev/trunk'
          [ https://svn.apache.org/r1717481 ]

          SOLR-8180: jcl-over-slf4j is officially a solrj/solr dependency now; not marked optional in a POM.

          Show
          ASF subversion and git services added a comment - Commit 1717481 from David Smiley in branch 'dev/trunk' [ https://svn.apache.org/r1717481 ] SOLR-8180 : jcl-over-slf4j is officially a solrj/solr dependency now; not marked optional in a POM.
          Hide
          ASF subversion and git services added a comment -

          Commit 1717483 from David Smiley in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1717483 ]

          SOLR-8180: jcl-over-slf4j is officially a solrj/solr dependency now; not marked optional in a POM.

          Show
          ASF subversion and git services added a comment - Commit 1717483 from David Smiley in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1717483 ] SOLR-8180 : jcl-over-slf4j is officially a solrj/solr dependency now; not marked optional in a POM.
          Hide
          ASF subversion and git services added a comment -

          Commit 1717497 from David Smiley in branch 'dev/branches/lucene_solr_5_4'
          [ https://svn.apache.org/r1717497 ]

          SOLR-8180: jcl-over-slf4j is officially a solrj/solr dependency now; not marked optional in a POM.

          Show
          ASF subversion and git services added a comment - Commit 1717497 from David Smiley in branch 'dev/branches/lucene_solr_5_4' [ https://svn.apache.org/r1717497 ] SOLR-8180 : jcl-over-slf4j is officially a solrj/solr dependency now; not marked optional in a POM.

            People

            • Assignee:
              David Smiley
              Reporter:
              Kevin Risden
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development