Solr
  1. Solr
  2. SOLR-1993

SolrJ binary update erro when commitWithin is set.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.4, 1.4.1
    • Fix Version/s: 1.4.2, 3.1, 4.0-ALPHA
    • Component/s: clients - java
    • Labels:
      None

      Description

      Solr server is unable to unmarshall a binary update request where the commitWithin property is set on the UpdateRequest class.

      The client marshalls the request with the following code
      if (updateRequest.getCommitWithin() != -1)

      { params.add("commitWithin", updateRequest.getCommitWithin()); }

      The property is an int and when the server unmarshalls, the following error happens (can't cast to List<String> due to an Integer element)

      SEVERE: java.lang.ClassCastException: java.lang.Integer cannot be cast to java.util.List
      at org.apache.solr.client.solrj.request.JavaBinUpdateRequestCodec.namedListToSolrParams(JavaBinUpdateRequestCodec.java:213)
      at org.apache.solr.client.solrj.request.JavaBinUpdateRequestCodec.access$100(JavaBinUpdateRequestCodec.java:40)
      at org.apache.solr.client.solrj.request.JavaBinUpdateRequestCodec$2.readOuterMostDocIterator(JavaBinUpdateRequestCodec.java:131)
      at org.apache.solr.client.solrj.request.JavaBinUpdateRequestCodec$2.readIterator(JavaBinUpdateRequestCodec.java:126)
      at org.apache.solr.common.util.JavaBinCodec.readVal(JavaBinCodec.java:210)
      at org.apache.solr.client.solrj.request.JavaBinUpdateRequestCodec$2.readNamedList(JavaBinUpdateRequestCodec.java:112)
      at org.apache.solr.common.util.JavaBinCodec.readVal(JavaBinCodec.java:175)
      at org.apache.solr.common.util.JavaBinCodec.unmarshal(JavaBinCodec.java:101)
      at org.apache.solr.client.solrj.request.JavaBinUpdateRequestCodec.unmarshal(JavaBinUpdateRequestCodec.java:141)
      at org.apache.solr.handler.BinaryUpdateRequestHandler.parseAndLoadDocs(BinaryUpdateRequestHandler.java:68)
      at org.apache.solr.handler.BinaryUpdateRequestHandler.access$000(BinaryUpdateRequestHandler.java:46)
      at org.apache.solr.handler.BinaryUpdateRequestHandler$1.load(BinaryUpdateRequestHandler.java:55)
      at org.apache.solr.handler.ContentStreamHandlerBase.handleRequestBody(ContentStreamHandlerBase.java:54)
      at org.apache.solr.handler.RequestHandlerBase.handleRequest(RequestHandlerBase.java:131)
      at org.apache.solr.core.SolrCore.execute(SolrCore.java:1316)
      at org.apache.solr.servlet.SolrDispatchFilter.execute(SolrDispatchFilter.java:338)
      at org.apache.solr.servlet.SolrDispatchFilter.doFilter(SolrDispatchFilter.java:241)
      at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235)
      at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206)
      at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:233)
      at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:191)
      at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:128)
      at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:102)
      at org.apache.catalina.valves.AccessLogValve.invoke(AccessLogValve.java:567)
      at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:109)
      at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:293)
      at org.apache.coyote.http11.Http11Processor.process(Http11Processor.java:849)
      at org.apache.coyote.http11.Http11Protocol$Http11ConnectionHandler.process(Http11Protocol.java:583)
      at org.apache.tomcat.util.net.JIoEndpoint$Worker.run(JIoEndpoint.java:454)
      at java.lang.Thread.run(Thread.java:619)

      Workaround is to set the parameter manually as a string value instead of setting using the property on the UpdateRequest class.

      1. SOLR-1993.patch
        4 kB
        Hoss Man
      2. SOLR-1993.patch
        3 kB
        Noble Paul
      3. SOLR-1993-1.4.patch
        4 kB
        Maxim Valyanskiy
      4. SolrExampleBinaryTest.java
        2 kB
        Maxim Valyanskiy

        Activity

        Hide
        Noble Paul added a comment -

        Added a constructor to ModifiableSolrParams(NamedList)

        Show
        Noble Paul added a comment - Added a constructor to ModifiableSolrParams(NamedList)
        Hide
        Noble Paul added a comment -

        untested patch

        Added a constructor to ModifiableSolrParams(NamedList)

        Show
        Noble Paul added a comment - untested patch Added a constructor to ModifiableSolrParams(NamedList)
        Hide
        Maxim Valyanskiy added a comment -

        Paul, could you show workaround code example?

        Show
        Maxim Valyanskiy added a comment - Paul, could you show workaround code example?
        Hide
        Noble Paul added a comment -

        This patch has the fix. There is no work around

        Show
        Noble Paul added a comment - This patch has the fix. There is no work around
        Hide
        Hoss Man added a comment -

        before committing this patch, I was going to modify one of the existing SolrJ tests to set commitWithin to demonstrate the bug (and then the fix) but i then discovered that SolrExampleTests already uses commitWithin...

            UpdateRequest up = new UpdateRequest();
            up.add( doc3 );
            up.setCommitWithin( 500 );  // a smaller commitWithin caused failures on the following assert
            up.process( server );
        

        ...and doesn't have this bug.

        i'm a little concerned about being able to verify that the fix actaully works w/o being able to reproduce the test – Nobel, can you explain why this test currently succeeds in spite of this bug? ideally could you include a test (or test additions) in the patch demonstrating the bug?

        Show
        Hoss Man added a comment - before committing this patch, I was going to modify one of the existing SolrJ tests to set commitWithin to demonstrate the bug (and then the fix) but i then discovered that SolrExampleTests already uses commitWithin... UpdateRequest up = new UpdateRequest(); up.add( doc3 ); up.setCommitWithin( 500 ); // a smaller commitWithin caused failures on the following assert up.process( server ); ...and doesn't have this bug. i'm a little concerned about being able to verify that the fix actaully works w/o being able to reproduce the test – Nobel, can you explain why this test currently succeeds in spite of this bug? ideally could you include a test (or test additions) in the patch demonstrating the bug?
        Hide
        Noble Paul added a comment -

        The Original code expects all params to be strings. In this case it is an integer that it's why it's failing.

        Show
        Noble Paul added a comment - The Original code expects all params to be strings. In this case it is an integer that it's why it's failing.
        Hide
        Hoss Man added a comment -

        The Original code expects all params to be strings. In this case it is an integer that it's why it's failing.

        I don't relaly see how that addresses my question at all.

        We have an existing test, which uses UpdateRequest.setCommitWithin, which uses an int, which passes.

        how can i actually verify this bug?

        Show
        Hoss Man added a comment - The Original code expects all params to be strings. In this case it is an integer that it's why it's failing. I don't relaly see how that addresses my question at all. We have an existing test, which uses UpdateRequest.setCommitWithin, which uses an int, which passes. how can i actually verify this bug?
        Hide
        Maxim Valyanskiy added a comment -

        I have the same problem. Unit tests derived from SolrExampleTests works because they do not use BinaryRequestWriter. I will write unit test for it tomorrow

        Show
        Maxim Valyanskiy added a comment - I have the same problem. Unit tests derived from SolrExampleTests works because they do not use BinaryRequestWriter. I will write unit test for it tomorrow
        Hide
        Maxim Valyanskiy added a comment -

        solr/src/test/org/apache/solr/client/solrj/embedded/SolrExampleBinaryTest.java

        Show
        Maxim Valyanskiy added a comment - solr/src/test/org/apache/solr/client/solrj/embedded/SolrExampleBinaryTest.java
        Hide
        Maxim Valyanskiy added a comment -

        this test reproduces the problem on todays lucene-solr trunk

        Show
        Maxim Valyanskiy added a comment - this test reproduces the problem on todays lucene-solr trunk
        Hide
        Hoss Man added a comment -

        Ah, thank you Maxim – not realizing that the existing Jetty tests use XML by default - that was the piece of the puzzle i was missing.

        With Maxim's test, i was able to reproduce the bug, and with Noble's patch the bug when away, but a new one still surfaced.

        However one of the things i noticed was that the SolrParam building code being moved around/fixed was in general redundant – we already have SolrParams.toSolrparams(NamedList) and using that made the tests pass

        So i switched to using that, and all tests now pass.

        attached patch include Maxim's test case and the fix – comments appreciated.

        Show
        Hoss Man added a comment - Ah, thank you Maxim – not realizing that the existing Jetty tests use XML by default - that was the piece of the puzzle i was missing. With Maxim's test, i was able to reproduce the bug, and with Noble's patch the bug when away, but a new one still surfaced. However one of the things i noticed was that the SolrParam building code being moved around/fixed was in general redundant – we already have SolrParams.toSolrparams(NamedList) and using that made the tests pass So i switched to using that, and all tests now pass. attached patch include Maxim's test case and the fix – comments appreciated.
        Hide
        Noble Paul added a comment -

        Oh. I missed that helper method. the patch is good

        Show
        Noble Paul added a comment - Oh. I missed that helper method. the patch is good
        Hide
        Maxim Valyanskiy added a comment -

        Patch for Solr 1.4 branch

        Show
        Maxim Valyanskiy added a comment - Patch for Solr 1.4 branch
        Hide
        Maxim Valyanskiy added a comment -

        I ported this patch to 1.4 branch and test it in my application. 5 min test passed without any problems, commitWithin parametes works as excepted.

        Is it possible to include this patch in 1.4.2?

        Show
        Maxim Valyanskiy added a comment - I ported this patch to 1.4 branch and test it in my application. 5 min test passed without any problems, commitWithin parametes works as excepted. Is it possible to include this patch in 1.4.2?
        Hide
        Hoss Man added a comment -

        Committed revision 1051611. - trunk
        Committed revision 1051616. - 3x
        Committed revision 1051624 - solr 1.4 branch

        Maxim: thanks for your help on this issue. I'm not sure if/when 1.4.2 might be released, but i went ahead and committed your backport just in case.

        Show
        Hoss Man added a comment - Committed revision 1051611. - trunk Committed revision 1051616. - 3x Committed revision 1051624 - solr 1.4 branch Maxim: thanks for your help on this issue. I'm not sure if/when 1.4.2 might be released, but i went ahead and committed your backport just in case.
        Hide
        Grant Ingersoll added a comment -

        Bulk close for 3.1.0 release

        Show
        Grant Ingersoll added a comment - Bulk close for 3.1.0 release

          People

          • Assignee:
            Hoss Man
            Reporter:
            Phil Bingley
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development