Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-9285

ArrayIndexOutOfBoundsException when ValueSourceAugmenter used with RTG on uncommitted doc

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.2, master (7.0)
    • Component/s: None
    • Security Level: Public (Default Security Level. Issues are Public)
    • Labels:
      None

      Description

      Found in SOLR-9180 testing.

      Even in single node solr envs, doing an RTG for an uncommitted doc that uses ValueSourceAugmenter (ie: simple field aliasing, or functions in fl) causes an ArrayIndexOutOfBoundsException

      1. SOLR-9285.patch
        49 kB
        Hoss Man
      2. SOLR-9285.patch
        41 kB
        Hoss Man
      3. SOLR-9285.patch
        37 kB
        Hoss Man
      4. SOLR-9285.patch
        30 kB
        Hoss Man
      5. SOLR-9285.patch
        10 kB
        Hoss Man

        Issue Links

          Activity

          Hide
          hossman Hoss Man added a comment -

          Assertion...

              // also check real-time-get from transaction log
              assertJQ(req("qt","/get", "id","99", "fl","val_ss:val_i, val2_ss:10, subject")
                       ,"/doc=={'val2_ss':10,'val_ss':1,'subject':'uncommitted'}"
              );
          

          Failure...

             [junit4] ERROR   0.06s J1 | TestPseudoReturnFields.testMultiValuedRTG <<<
             [junit4]    > Throwable #1: java.lang.ArrayIndexOutOfBoundsException: -1
             [junit4]    > 	at __randomizedtesting.SeedInfo.seed([35D25075C14EB91B:4D9722282B16D95D]:0)
             [junit4]    > 	at java.util.ArrayList.elementData(ArrayList.java:418)
             [junit4]    > 	at java.util.ArrayList.get(ArrayList.java:431)
             [junit4]    > 	at java.util.Collections$UnmodifiableList.get(Collections.java:1309)
             [junit4]    > 	at org.apache.solr.response.transform.ValueSourceAugmenter.transform(ValueSourceAugmenter.java:94)
             [junit4]    > 	at org.apache.solr.response.transform.DocTransformers.transform(DocTransformers.java:76)
             [junit4]    > 	at org.apache.solr.handler.component.RealTimeGetComponent.process(RealTimeGetComponent.java:219)
             [junit4]    > 	at org.apache.solr.handler.component.SearchHandler.handleRequestBody(SearchHandler.java:292)
             [junit4]    > 	at org.apache.solr.handler.RequestHandlerBase.handleRequest(RequestHandlerBase.java:154)
             [junit4]    > 	at org.apache.solr.core.SolrCore.execute(SolrCore.java:2035)
             [junit4]    > 	at org.apache.solr.util.TestHarness.query(TestHarness.java:310)
             [junit4]    > 	at org.apache.solr.util.TestHarness.query(TestHarness.java:292)
             [junit4]    > 	at org.apache.solr.SolrTestCaseJ4.assertJQ(SolrTestCaseJ4.java:846)
             [junit4]    > 	at org.apache.solr.SolrTestCaseJ4.assertJQ(SolrTestCaseJ4.java:815)
             [junit4]    > 	at org.apache.solr.search.TestPseudoReturnFields.testMultiValuedRTG(TestPseudoReturnFields.java:108)
          
          Show
          hossman Hoss Man added a comment - Assertion... // also check real-time-get from transaction log assertJQ(req( "qt" , "/get" , "id" , "99" , "fl" , "val_ss:val_i, val2_ss:10, subject" ) , "/doc=={'val2_ss':10,'val_ss':1,'subject':'uncommitted'}" ); Failure... [junit4] ERROR 0.06s J1 | TestPseudoReturnFields.testMultiValuedRTG <<< [junit4] > Throwable #1: java.lang.ArrayIndexOutOfBoundsException: -1 [junit4] > at __randomizedtesting.SeedInfo.seed([35D25075C14EB91B:4D9722282B16D95D]:0) [junit4] > at java.util.ArrayList.elementData(ArrayList.java:418) [junit4] > at java.util.ArrayList.get(ArrayList.java:431) [junit4] > at java.util.Collections$UnmodifiableList.get(Collections.java:1309) [junit4] > at org.apache.solr.response.transform.ValueSourceAugmenter.transform(ValueSourceAugmenter.java:94) [junit4] > at org.apache.solr.response.transform.DocTransformers.transform(DocTransformers.java:76) [junit4] > at org.apache.solr.handler.component.RealTimeGetComponent.process(RealTimeGetComponent.java:219) [junit4] > at org.apache.solr.handler.component.SearchHandler.handleRequestBody(SearchHandler.java:292) [junit4] > at org.apache.solr.handler.RequestHandlerBase.handleRequest(RequestHandlerBase.java:154) [junit4] > at org.apache.solr.core.SolrCore.execute(SolrCore.java:2035) [junit4] > at org.apache.solr.util.TestHarness.query(TestHarness.java:310) [junit4] > at org.apache.solr.util.TestHarness.query(TestHarness.java:292) [junit4] > at org.apache.solr.SolrTestCaseJ4.assertJQ(SolrTestCaseJ4.java:846) [junit4] > at org.apache.solr.SolrTestCaseJ4.assertJQ(SolrTestCaseJ4.java:815) [junit4] > at org.apache.solr.search.TestPseudoReturnFields.testMultiValuedRTG(TestPseudoReturnFields.java:108)
          Hide
          hossman Hoss Man added a comment -

          See also...

          • TestPseudoReturnFields.testFunctionsRTG
          • TestPseudoReturnFields.testFunctionsAndExplicitRTG
          Show
          hossman Hoss Man added a comment - See also... TestPseudoReturnFields.testFunctionsRTG TestPseudoReturnFields.testFunctionsAndExplicitRTG
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit fee9526208375fec6a7651249b182fbca1a29703 in lucene-solr's branch refs/heads/branch_6x from Chris Hostetter
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=fee9526 ]

          SOLR-9180: More comprehensive tests of psuedo-fields for RTG and SolrCloud requests

          This commit also includes new @AwaitsFix'ed tests for the following known issues...

          • SOLR-9285 ArrayIndexOutOfBoundsException when ValueSourceAugmenter used with RTG on uncommitted doc
          • SOLR-9286 SolrCloud RTG: psuedo-fields (like ValueSourceAugmenter, [shard], etc...) silently fails (even for committed doc)
          • SOLR-9287 single node RTG: NPE if score is requested
          • SOLR-9288 RTG: fl=[docid] silently missing for uncommitted docs
          • SOLR-9289 SolrCloud RTG: fl=[docid] silently ignored for all docs

          (cherry picked from commit ae316f1e39e58d89758f997913a38059d74ccb47)

          Show
          jira-bot ASF subversion and git services added a comment - Commit fee9526208375fec6a7651249b182fbca1a29703 in lucene-solr's branch refs/heads/branch_6x from Chris Hostetter [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=fee9526 ] SOLR-9180 : More comprehensive tests of psuedo-fields for RTG and SolrCloud requests This commit also includes new @AwaitsFix'ed tests for the following known issues... SOLR-9285 ArrayIndexOutOfBoundsException when ValueSourceAugmenter used with RTG on uncommitted doc SOLR-9286 SolrCloud RTG: psuedo-fields (like ValueSourceAugmenter, [shard] , etc...) silently fails (even for committed doc) SOLR-9287 single node RTG: NPE if score is requested SOLR-9288 RTG: fl= [docid] silently missing for uncommitted docs SOLR-9289 SolrCloud RTG: fl= [docid] silently ignored for all docs (cherry picked from commit ae316f1e39e58d89758f997913a38059d74ccb47)
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit ae316f1e39e58d89758f997913a38059d74ccb47 in lucene-solr's branch refs/heads/master from Chris Hostetter
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=ae316f1 ]

          SOLR-9180: More comprehensive tests of psuedo-fields for RTG and SolrCloud requests

          This commit also includes new @AwaitsFix'ed tests for the following known issues...

          • SOLR-9285 ArrayIndexOutOfBoundsException when ValueSourceAugmenter used with RTG on uncommitted doc
          • SOLR-9286 SolrCloud RTG: psuedo-fields (like ValueSourceAugmenter, [shard], etc...) silently fails (even for committed doc)
          • SOLR-9287 single node RTG: NPE if score is requested
          • SOLR-9288 RTG: fl=[docid] silently missing for uncommitted docs
          • SOLR-9289 SolrCloud RTG: fl=[docid] silently ignored for all docs
          Show
          jira-bot ASF subversion and git services added a comment - Commit ae316f1e39e58d89758f997913a38059d74ccb47 in lucene-solr's branch refs/heads/master from Chris Hostetter [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=ae316f1 ] SOLR-9180 : More comprehensive tests of psuedo-fields for RTG and SolrCloud requests This commit also includes new @AwaitsFix'ed tests for the following known issues... SOLR-9285 ArrayIndexOutOfBoundsException when ValueSourceAugmenter used with RTG on uncommitted doc SOLR-9286 SolrCloud RTG: psuedo-fields (like ValueSourceAugmenter, [shard] , etc...) silently fails (even for committed doc) SOLR-9287 single node RTG: NPE if score is requested SOLR-9288 RTG: fl= [docid] silently missing for uncommitted docs SOLR-9289 SolrCloud RTG: fl= [docid] silently ignored for all docs
          Hide
          hossman Hoss Man added a comment -

          The root cause of the AIOOBE is that when a doc is found in the UpdateLog, the RTG component explicitly passes -1 to DocTransformer.transform(), and currently ValueSourceAugmenter.transform() doesn't account for this possibility when using it's IndexReader to lookup the FunctionValues for the document.

          Obviously, even if it did do some sanity checking for negative docids, that wouldn't really address the root of the problem – getting usable FunctionValues.

          The RTG Component already has a code path where it sometimes uses ulog.openRealtimeSearcher() when doc are found in the update log, instead of returning the document directly from the ulog command – but that code path is currently confined to situations where fq params are included in the RTG request (so it can evaluate them against the Realtime Searcher to ensure the doc should really be returned)


          I'm attaching a straw man patch that:

          • refactors the ulog.openRealtimeSearcher() logic in RTG Component to also apply when there are transformers
          • Adds a new light weight (private) RTGResultContext class for wrapping realtime {{SolrIndexSearcher}}s for use with transformers.
          • Fixes ValueSourceAugmenter.setContext to use ResultContext.getSearcher() instead of the one that comes from the request
            • Independent of anything else, this seems like a bug (it certainly violates the javadocs of ResultContext.getRequest(), even if it was getting the same SolrQueryRequest via a more round about way)

          This patch fixes all existing @AwaitsFix(SOLR-9285) tests, and doesn't seem to cause any new test failures in the code base, but i don't think we should commit it as is for the simple reason that it seems like it is overkill: Simple transformers like RenameFieldTransformer can operate just fine on docs from the UpdateLog w/o any need for a (realtime) SolrIndexSearcher.

          As noted in the patch with a nocommit I think what would be best is to enahnce this patch by adding a boolean needsSolrIndexSearcher() method to the DocTransformer API, which defaults to false but any transformers like ValueSourceAugmenter that actually need access to a SolrIndexSearcher in their setContext method could ask for it, and the code in this patch that sets mustUseRealtimeSearcher = true anytime there is a transformer could be modified to instead use transformer.needsSolrIndexSearcher()

          Questions/comments/concerns?


          Reviewing the RTG Component code also makes me realize that in general we should have more RTG+transformer tests which:

          • use fq params
          • ask for uncommited ids both immediately after adding them (before a realtime searcher may have been opened) AND "after" a realtime searcher has been opened (for whatever reason)
          • ask for multiple ids in a single request (to ensure nothing broken in the looping logic)
            • mixing commited, uncommited, and uncommited but in an already re-opened realtime searcher
            • mixing them in various orders, since that affects when a realtime searcher might be opened
          Show
          hossman Hoss Man added a comment - The root cause of the AIOOBE is that when a doc is found in the UpdateLog , the RTG component explicitly passes -1 to DocTransformer.transform() , and currently ValueSourceAugmenter.transform() doesn't account for this possibility when using it's IndexReader to lookup the FunctionValues for the document. Obviously, even if it did do some sanity checking for negative docids, that wouldn't really address the root of the problem – getting usable FunctionValues. The RTG Component already has a code path where it sometimes uses ulog.openRealtimeSearcher() when doc are found in the update log, instead of returning the document directly from the ulog command – but that code path is currently confined to situations where fq params are included in the RTG request (so it can evaluate them against the Realtime Searcher to ensure the doc should really be returned) I'm attaching a straw man patch that: refactors the ulog.openRealtimeSearcher() logic in RTG Component to also apply when there are transformers Adds a new light weight (private) RTGResultContext class for wrapping realtime {{SolrIndexSearcher}}s for use with transformers. Fixes ValueSourceAugmenter.setContext to use ResultContext.getSearcher() instead of the one that comes from the request Independent of anything else, this seems like a bug (it certainly violates the javadocs of ResultContext.getRequest() , even if it was getting the same SolrQueryRequest via a more round about way) This patch fixes all existing @AwaitsFix( SOLR-9285 ) tests, and doesn't seem to cause any new test failures in the code base, but i don't think we should commit it as is for the simple reason that it seems like it is overkill: Simple transformers like RenameFieldTransformer can operate just fine on docs from the UpdateLog w/o any need for a (realtime) SolrIndexSearcher. As noted in the patch with a nocommit I think what would be best is to enahnce this patch by adding a boolean needsSolrIndexSearcher() method to the DocTransformer API, which defaults to false but any transformers like ValueSourceAugmenter that actually need access to a SolrIndexSearcher in their setContext method could ask for it, and the code in this patch that sets mustUseRealtimeSearcher = true anytime there is a transformer could be modified to instead use transformer.needsSolrIndexSearcher() Questions/comments/concerns? Reviewing the RTG Component code also makes me realize that in general we should have more RTG+transformer tests which: use fq params ask for uncommited ids both immediately after adding them (before a realtime searcher may have been opened) AND "after" a realtime searcher has been opened (for whatever reason) ask for multiple ids in a single request (to ensure nothing broken in the looping logic) mixing commited, uncommited, and uncommited but in an already re-opened realtime searcher mixing them in various orders, since that affects when a realtime searcher might be opened
          Hide
          dsmiley David Smiley added a comment -

          Your plan makes sense to me Hoss. Thanks for your thoroughness.

          Show
          dsmiley David Smiley added a comment - Your plan makes sense to me Hoss. Thanks for your thoroughness.
          Hide
          hossman Hoss Man added a comment -

          Reviewing the RTG Component code also makes me realize that in general we should have more RTG+transformer tests which:

          updated patch with a randomized test along these lines ... still some holes to fill in, but mostly feature complete.

          Show
          hossman Hoss Man added a comment - Reviewing the RTG Component code also makes me realize that in general we should have more RTG+transformer tests which: updated patch with a randomized test along these lines ... still some holes to fill in, but mostly feature complete.
          Hide
          yseeley@gmail.com Yonik Seeley added a comment -
          • refactors the ulog.openRealtimeSearcher() logic in RTG Component to also apply when there are transformers
          • Adds a new light weight (private) RTGResultContext class for wrapping realtime {{SolrIndexSearcher}}s for use with transformers.

          +1, this sounds like the right approach.

          • Fixes ValueSourceAugmenter.setContext to use ResultContext.getSearcher() instead of the one that comes from the request
            Independent of anything else, this seems like a bug

          Yep. ResultContext having a getSearcher is relatively new... I added it to support SOLR-7830, but I obviously didn't find all the places that needed changing.

          Show
          yseeley@gmail.com Yonik Seeley added a comment - refactors the ulog.openRealtimeSearcher() logic in RTG Component to also apply when there are transformers Adds a new light weight (private) RTGResultContext class for wrapping realtime {{SolrIndexSearcher}}s for use with transformers. +1, this sounds like the right approach. Fixes ValueSourceAugmenter.setContext to use ResultContext.getSearcher() instead of the one that comes from the request Independent of anything else, this seems like a bug Yep. ResultContext having a getSearcher is relatively new... I added it to support SOLR-7830 , but I obviously didn't find all the places that needed changing.
          Hide
          hossman Hoss Man added a comment -

          Heavily beefed up test, nearly finished.

          Uncovered SOLR-9308 & SOLR-9309 in the process – currently just working around them with commented out TODOs in test since they are secondary to the primary goal here.

          Show
          hossman Hoss Man added a comment - Heavily beefed up test, nearly finished. Uncovered SOLR-9308 & SOLR-9309 in the process – currently just working around them with commented out TODOs in test since they are secondary to the primary goal here.
          Hide
          hossman Hoss Man added a comment -

          Updated patch:

          • fully fleshed out TestRandomFlRTGCloud
          • added the previously proposed needsSolrIndexSearcher method to DocTransformer
            • default impl always returns false
            • ValueSourceAugmenter overrides to always returns true
            • DocTransformers overrides to return true if any of the wrapped/child transformers return true

          There are almost certainly some other DocTransformer subclasses that should return true from this method, but I'd like to move forward with committing as is and target fixing any other classes (and adding test coverage for them) in other issues. (As things stand in this patch they are no worse off then before.)

          Unless there are objections, I'll try to commit/backport this on monday. (possibly after filing some new issues so i can update some TODO comments in tests with concrete jira IDs)

          Show
          hossman Hoss Man added a comment - Updated patch: fully fleshed out TestRandomFlRTGCloud added the previously proposed needsSolrIndexSearcher method to DocTransformer default impl always returns false ValueSourceAugmenter overrides to always returns true DocTransformers overrides to return true if any of the wrapped/child transformers return true There are almost certainly some other DocTransformer subclasses that should return true from this method, but I'd like to move forward with committing as is and target fixing any other classes (and adding test coverage for them) in other issues. (As things stand in this patch they are no worse off then before.) Unless there are objections, I'll try to commit/backport this on monday. (possibly after filing some new issues so i can update some TODO comments in tests with concrete jira IDs)
          Hide
          hossman Hoss Man added a comment -

          Minor patch updates after doing a re-read this morning:

          • Updated some TODO comments to refer to (newly created) SOLR-9314
          • Updated TestPseudoReturnFields to replace some TODO comments with more coverage of abs(val_i) now that it works.

          ...committing & backporting now.

          Show
          hossman Hoss Man added a comment - Minor patch updates after doing a re-read this morning: Updated some TODO comments to refer to (newly created) SOLR-9314 Updated TestPseudoReturnFields to replace some TODO comments with more coverage of abs(val_i) now that it works. ...committing & backporting now.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 9bde118c2f755ffc060b893248f64ff7da9400c2 in lucene-solr's branch refs/heads/branch_6x from Chris Hostetter
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=9bde118 ]

          SOLR-9285: Fixed AIOOBE when using ValueSourceAugmenter in single node RTG

          (cherry picked from commit 4123b3bf26156227174ef3c417b36309c2beeb9a)

          Show
          jira-bot ASF subversion and git services added a comment - Commit 9bde118c2f755ffc060b893248f64ff7da9400c2 in lucene-solr's branch refs/heads/branch_6x from Chris Hostetter [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=9bde118 ] SOLR-9285 : Fixed AIOOBE when using ValueSourceAugmenter in single node RTG (cherry picked from commit 4123b3bf26156227174ef3c417b36309c2beeb9a)
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 4123b3bf26156227174ef3c417b36309c2beeb9a in lucene-solr's branch refs/heads/master from Chris Hostetter
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=4123b3b ]

          SOLR-9285: Fixed AIOOBE when using ValueSourceAugmenter in single node RTG

          Show
          jira-bot ASF subversion and git services added a comment - Commit 4123b3bf26156227174ef3c417b36309c2beeb9a in lucene-solr's branch refs/heads/master from Chris Hostetter [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=4123b3b ] SOLR-9285 : Fixed AIOOBE when using ValueSourceAugmenter in single node RTG
          Hide
          mikemccand Michael McCandless added a comment -

          Bulk close resolved issues after 6.2.0 release.

          Show
          mikemccand Michael McCandless added a comment - Bulk close resolved issues after 6.2.0 release.

            People

            • Assignee:
              hossman Hoss Man
              Reporter:
              hossman Hoss Man
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development