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

Mismatched Docvalue segments cause exception in Sorting/Facting; Uninvert per segment

    Details

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

      Description

      The configuration of UninvertingReader in SolrIndexSearch creates a global mapping for the directory for fields to uninvert. If docvalues are enabled on a field the creation of a new segment will cause the query to fail when faceting/sorting on the recently docvalue enabled field. This happens because the UninvertingReader is configured globally across the entire directory, and a single segment containing DVs for a field will incorrectly indicate that all segments contain DVs.

      This patch addresses the incorrect behavior by determining the fields to be uninverted on a per-segment basis.

      With the fix, it is still recommended that a reindexing occur as data loss will when a DV and non-DV segment are merged, SOLR-10046 addresses this behavior. This fix is to be a stop gap for the time between enabling docvalues and the duration of a reindex.

      1. SOLR_10047_test.patch
        10 kB
        Shalin Shekhar Mangar

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user kelaban opened a pull request:

          https://github.com/apache/lucene-solr/pull/145

          SOLR-10047 - SolrIndexSearcher, UninvertingReader, uninvert docvalues…

          … per segment

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/kelaban/lucene-solr jira/master/SOLR-10047/uninverting-reader-per-segment

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/lucene-solr/pull/145.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #145


          commit 2898c46613f2708ce8c472f821ac4f2c42cd8b48
          Author: Keith Laban <klaban1@bloomberg.net>
          Date: 2017-01-18T21:39:51Z

          SOLR-10047 - SolrIndexSearcher, UninvertingReader, uninvert docvalues per segment


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user kelaban opened a pull request: https://github.com/apache/lucene-solr/pull/145 SOLR-10047 - SolrIndexSearcher, UninvertingReader, uninvert docvalues… … per segment You can merge this pull request into a Git repository by running: $ git pull https://github.com/kelaban/lucene-solr jira/master/ SOLR-10047 /uninverting-reader-per-segment Alternatively you can review and apply these changes as the patch at: https://github.com/apache/lucene-solr/pull/145.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #145 commit 2898c46613f2708ce8c472f821ac4f2c42cd8b48 Author: Keith Laban <klaban1@bloomberg.net> Date: 2017-01-18T21:39:51Z SOLR-10047 - SolrIndexSearcher, UninvertingReader, uninvert docvalues per segment
          Hide
          shalinmangar Shalin Shekhar Mangar added a comment - - edited

          Thanks Keith.

          • I merged the pull request to latest master to try this out locally and the UninvertingDirectoryReaderMappingPerSegment requires an implementation of IndexReader#getReaderCacheHelper method which was added to IndexReader in LUCENE-7410
          • I find the javadocs for UninvertingDirectoryReaderMappingPerSegment a little confusing. I made an attempt to rewrite them to convey the intention and implementation better. Use if you see fit:
               * If docvalues are enabled or disabled after data has already been indexed for a field, such that
               * only some segments have docvalues, uninverting on the top level reader will cause 
               * IllegalStateException to be thrown when trying to use a field with such mixed data. This is because
               * the {@link IndexSchema#getUninversionMap(IndexReader)} method decides to put a field 
               * into the uninverteding map only if *NO* segment in the index contains docvalues for that field.
               * 
               * Therefore, this class provides a uninverting map per segment such that for any field, 
               * DocValues are used from segments if they exist and uninversion of the field is performed on the rest
               * of the segments.
            
          • Perhaps we can rename UninvertingDirectoryReaderMappingPerSegment to PerSegmentUninvertingDirectoryReader or UninvertingDirectoryReaderPerSegmentMapping?

          The rest of the changes looks good to me.

          Show
          shalinmangar Shalin Shekhar Mangar added a comment - - edited Thanks Keith. I merged the pull request to latest master to try this out locally and the UninvertingDirectoryReaderMappingPerSegment requires an implementation of IndexReader#getReaderCacheHelper method which was added to IndexReader in LUCENE-7410 I find the javadocs for UninvertingDirectoryReaderMappingPerSegment a little confusing. I made an attempt to rewrite them to convey the intention and implementation better. Use if you see fit: * If docvalues are enabled or disabled after data has already been indexed for a field, such that * only some segments have docvalues, uninverting on the top level reader will cause * IllegalStateException to be thrown when trying to use a field with such mixed data. This is because * the {@link IndexSchema#getUninversionMap(IndexReader)} method decides to put a field * into the uninverteding map only if *NO* segment in the index contains docvalues for that field. * * Therefore, this class provides a uninverting map per segment such that for any field, * DocValues are used from segments if they exist and uninversion of the field is performed on the rest * of the segments. Perhaps we can rename UninvertingDirectoryReaderMappingPerSegment to PerSegmentUninvertingDirectoryReader or UninvertingDirectoryReaderPerSegmentMapping? The rest of the changes looks good to me.
          Hide
          k317h Keith Laban added a comment -
          • updated javadoc
          • renamed class to UninvertingDirectoryReaderPerSegmentMapping
          • added IndexReader#getReaderCacheHelper (copied note from UninvertingDirectoryReader)
          • removed old now unsued UninvertingDirectoryReader
          Show
          k317h Keith Laban added a comment - updated javadoc renamed class to UninvertingDirectoryReaderPerSegmentMapping added IndexReader#getReaderCacheHelper (copied note from UninvertingDirectoryReader) removed old now unsued UninvertingDirectoryReader
          Hide
          shalinmangar Shalin Shekhar Mangar added a comment -

          Hi Keith, the UninvertingReader#wrap method is used in multiple tests so we can't remove it. We should audit the uses and use UninvertingDirectoryReaderPerSegmentMapping.

          Show
          shalinmangar Shalin Shekhar Mangar added a comment - Hi Keith, the UninvertingReader#wrap method is used in multiple tests so we can't remove it. We should audit the uses and use UninvertingDirectoryReaderPerSegmentMapping.
          Hide
          k317h Keith Laban added a comment -

          Shalin, based on your comment I did a bit of an overhaul to the original PR.

          Instead of deleting UninvertingReader#wrap I changed the interface from accepting Map to Function<LeafReader,Map>. With this change I updated SolrIndexSearcher to use this interface instead of creating a new static class. I also updated all of the places where the original static function wrap was being used.

          If you think updating all of the test to the new interface is too much overkill/not backcompat I can overload the UninvertingReader#wrap function to accept the original static mapping and delegate to the new impl.

          Show
          k317h Keith Laban added a comment - Shalin, based on your comment I did a bit of an overhaul to the original PR. Instead of deleting UninvertingReader#wrap I changed the interface from accepting Map to Function<LeafReader,Map> . With this change I updated SolrIndexSearcher to use this interface instead of creating a new static class. I also updated all of the places where the original static function wrap was being used. If you think updating all of the test to the new interface is too much overkill/not backcompat I can overload the UninvertingReader#wrap function to accept the original static mapping and delegate to the new impl.
          Hide
          shalinmangar Shalin Shekhar Mangar added a comment -

          Thanks Keith, I think we need to keep the old wrap function around for backcompat in 6.x but it should be fine to remove it in the master branch. Rest looks good to me.

          Show
          shalinmangar Shalin Shekhar Mangar added a comment - Thanks Keith, I think we need to keep the old wrap function around for backcompat in 6.x but it should be fine to remove it in the master branch. Rest looks good to me.
          Hide
          k317h Keith Laban added a comment -

          Shalin, I wasn't sure exactly how you wanted me to submit the diff between 6.x and master.

          In this PR:

          • Commit adds new and old interface, and modifies all the tests to use the new interface
          • Commit reverts the tests to using the old interface.

          If you want to keep both interfaces as a convenience method and tests unmodified you can squash them all down. Otherwise use HEAD for 6.x (tests not updated) and reset to HEAD^ for master (tests updated).

          I did not write specific tests to explicitly use new/old signature because they all use the new signature under the hood. The test I added in the original commit tests the updated intended behavior.

          Show
          k317h Keith Laban added a comment - Shalin, I wasn't sure exactly how you wanted me to submit the diff between 6.x and master. In this PR: Commit adds new and old interface, and modifies all the tests to use the new interface Commit reverts the tests to using the old interface. If you want to keep both interfaces as a convenience method and tests unmodified you can squash them all down. Otherwise use HEAD for 6.x (tests not updated) and reset to HEAD^ for master (tests updated). I did not write specific tests to explicitly use new/old signature because they all use the new signature under the hood. The test I added in the original commit tests the updated intended behavior.
          Hide
          shalinmangar Shalin Shekhar Mangar added a comment -

          Thanks Keith. Let's keep both interfaces and the tests unmodified. I'll squash merge your pull request to master and backport to 6x.

          Show
          shalinmangar Shalin Shekhar Mangar added a comment - Thanks Keith. Let's keep both interfaces and the tests unmodified. I'll squash merge your pull request to master and backport to 6x.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 4da901a0728239ac4e87b662533f966158991948 in lucene-solr's branch refs/heads/master from Shalin Shekhar Mangar
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=4da901a ]

          SOLR-10047: Mismatched Docvalues segments cause exception in Sorting/Faceting. Solr now uninverts per segment to avoid such exceptions

          Squashed commit of the following:

          commit c38f4cabc2828ee83b53b931dd829e29a3e1701c
          Author: Keith Laban <kelaban17@gmail.com>
          Date: Tue Apr 11 17:17:05 2017 -0400

          reverted tests to using old wrap interface

          commit 806f33e092491cc6a2ee292d2934c76171e40dc7
          Author: Keith Laban <kelaban17@gmail.com>
          Date: Tue Apr 11 17:13:34 2017 -0400

          updated UninvertingReader.wrap / tests

          commit b10bcab338b362b909491fea1cf13de66f5f17c0
          Author: Keith Laban <klaban1@bloomberg.net>
          Date: Wed Apr 5 14:57:28 2017 -0400

          SOLR-10047 - Updated javadoc/renamed class/added getReaderCacheHelper

          commit 90ecf5a4ae4feaf3efc42a1ed8643ad21e1c73ce
          Author: Keith Laban <klaban1@bloomberg.net>
          Date: Wed Jan 18 16:39:51 2017 -0500

          SOLR-10047 - SolrIndexSearcher, UninvertingReader, uninvert docvalues per segment

          Show
          jira-bot ASF subversion and git services added a comment - Commit 4da901a0728239ac4e87b662533f966158991948 in lucene-solr's branch refs/heads/master from Shalin Shekhar Mangar [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=4da901a ] SOLR-10047 : Mismatched Docvalues segments cause exception in Sorting/Faceting. Solr now uninverts per segment to avoid such exceptions Squashed commit of the following: commit c38f4cabc2828ee83b53b931dd829e29a3e1701c Author: Keith Laban <kelaban17@gmail.com> Date: Tue Apr 11 17:17:05 2017 -0400 reverted tests to using old wrap interface commit 806f33e092491cc6a2ee292d2934c76171e40dc7 Author: Keith Laban <kelaban17@gmail.com> Date: Tue Apr 11 17:13:34 2017 -0400 updated UninvertingReader.wrap / tests commit b10bcab338b362b909491fea1cf13de66f5f17c0 Author: Keith Laban <klaban1@bloomberg.net> Date: Wed Apr 5 14:57:28 2017 -0400 SOLR-10047 - Updated javadoc/renamed class/added getReaderCacheHelper commit 90ecf5a4ae4feaf3efc42a1ed8643ad21e1c73ce Author: Keith Laban <klaban1@bloomberg.net> Date: Wed Jan 18 16:39:51 2017 -0500 SOLR-10047 - SolrIndexSearcher, UninvertingReader, uninvert docvalues per segment
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 4da901a0728239ac4e87b662533f966158991948 in lucene-solr's branch refs/heads/master from Shalin Shekhar Mangar
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=4da901a ]

          SOLR-10047: Mismatched Docvalues segments cause exception in Sorting/Faceting. Solr now uninverts per segment to avoid such exceptions

          Squashed commit of the following:

          commit c38f4cabc2828ee83b53b931dd829e29a3e1701c
          Author: Keith Laban <kelaban17@gmail.com>
          Date: Tue Apr 11 17:17:05 2017 -0400

          reverted tests to using old wrap interface

          commit 806f33e092491cc6a2ee292d2934c76171e40dc7
          Author: Keith Laban <kelaban17@gmail.com>
          Date: Tue Apr 11 17:13:34 2017 -0400

          updated UninvertingReader.wrap / tests

          commit b10bcab338b362b909491fea1cf13de66f5f17c0
          Author: Keith Laban <klaban1@bloomberg.net>
          Date: Wed Apr 5 14:57:28 2017 -0400

          SOLR-10047 - Updated javadoc/renamed class/added getReaderCacheHelper

          commit 90ecf5a4ae4feaf3efc42a1ed8643ad21e1c73ce
          Author: Keith Laban <klaban1@bloomberg.net>
          Date: Wed Jan 18 16:39:51 2017 -0500

          SOLR-10047 - SolrIndexSearcher, UninvertingReader, uninvert docvalues per segment

          Show
          jira-bot ASF subversion and git services added a comment - Commit 4da901a0728239ac4e87b662533f966158991948 in lucene-solr's branch refs/heads/master from Shalin Shekhar Mangar [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=4da901a ] SOLR-10047 : Mismatched Docvalues segments cause exception in Sorting/Faceting. Solr now uninverts per segment to avoid such exceptions Squashed commit of the following: commit c38f4cabc2828ee83b53b931dd829e29a3e1701c Author: Keith Laban <kelaban17@gmail.com> Date: Tue Apr 11 17:17:05 2017 -0400 reverted tests to using old wrap interface commit 806f33e092491cc6a2ee292d2934c76171e40dc7 Author: Keith Laban <kelaban17@gmail.com> Date: Tue Apr 11 17:13:34 2017 -0400 updated UninvertingReader.wrap / tests commit b10bcab338b362b909491fea1cf13de66f5f17c0 Author: Keith Laban <klaban1@bloomberg.net> Date: Wed Apr 5 14:57:28 2017 -0400 SOLR-10047 - Updated javadoc/renamed class/added getReaderCacheHelper commit 90ecf5a4ae4feaf3efc42a1ed8643ad21e1c73ce Author: Keith Laban <klaban1@bloomberg.net> Date: Wed Jan 18 16:39:51 2017 -0500 SOLR-10047 - SolrIndexSearcher, UninvertingReader, uninvert docvalues per segment
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 4da901a0728239ac4e87b662533f966158991948 in lucene-solr's branch refs/heads/master from Shalin Shekhar Mangar
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=4da901a ]

          SOLR-10047: Mismatched Docvalues segments cause exception in Sorting/Faceting. Solr now uninverts per segment to avoid such exceptions

          Squashed commit of the following:

          commit c38f4cabc2828ee83b53b931dd829e29a3e1701c
          Author: Keith Laban <kelaban17@gmail.com>
          Date: Tue Apr 11 17:17:05 2017 -0400

          reverted tests to using old wrap interface

          commit 806f33e092491cc6a2ee292d2934c76171e40dc7
          Author: Keith Laban <kelaban17@gmail.com>
          Date: Tue Apr 11 17:13:34 2017 -0400

          updated UninvertingReader.wrap / tests

          commit b10bcab338b362b909491fea1cf13de66f5f17c0
          Author: Keith Laban <klaban1@bloomberg.net>
          Date: Wed Apr 5 14:57:28 2017 -0400

          SOLR-10047 - Updated javadoc/renamed class/added getReaderCacheHelper

          commit 90ecf5a4ae4feaf3efc42a1ed8643ad21e1c73ce
          Author: Keith Laban <klaban1@bloomberg.net>
          Date: Wed Jan 18 16:39:51 2017 -0500

          SOLR-10047 - SolrIndexSearcher, UninvertingReader, uninvert docvalues per segment

          Show
          jira-bot ASF subversion and git services added a comment - Commit 4da901a0728239ac4e87b662533f966158991948 in lucene-solr's branch refs/heads/master from Shalin Shekhar Mangar [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=4da901a ] SOLR-10047 : Mismatched Docvalues segments cause exception in Sorting/Faceting. Solr now uninverts per segment to avoid such exceptions Squashed commit of the following: commit c38f4cabc2828ee83b53b931dd829e29a3e1701c Author: Keith Laban <kelaban17@gmail.com> Date: Tue Apr 11 17:17:05 2017 -0400 reverted tests to using old wrap interface commit 806f33e092491cc6a2ee292d2934c76171e40dc7 Author: Keith Laban <kelaban17@gmail.com> Date: Tue Apr 11 17:13:34 2017 -0400 updated UninvertingReader.wrap / tests commit b10bcab338b362b909491fea1cf13de66f5f17c0 Author: Keith Laban <klaban1@bloomberg.net> Date: Wed Apr 5 14:57:28 2017 -0400 SOLR-10047 - Updated javadoc/renamed class/added getReaderCacheHelper commit 90ecf5a4ae4feaf3efc42a1ed8643ad21e1c73ce Author: Keith Laban <klaban1@bloomberg.net> Date: Wed Jan 18 16:39:51 2017 -0500 SOLR-10047 - SolrIndexSearcher, UninvertingReader, uninvert docvalues per segment
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 3e4629b8dfc6942fe391b741a8fb7d7baaeb854d in lucene-solr's branch refs/heads/branch_6x from Shalin Shekhar Mangar
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=3e4629b ]

          SOLR-10047: Mismatched Docvalues segments cause exception in Sorting/Faceting. Solr now uninverts per segment to avoid such exceptions

          Squashed commit of the following:

          commit c38f4cabc2828ee83b53b931dd829e29a3e1701c
          Author: Keith Laban <kelaban17@gmail.com>
          Date: Tue Apr 11 17:17:05 2017 -0400

          reverted tests to using old wrap interface

          commit 806f33e092491cc6a2ee292d2934c76171e40dc7
          Author: Keith Laban <kelaban17@gmail.com>
          Date: Tue Apr 11 17:13:34 2017 -0400

          updated UninvertingReader.wrap / tests

          commit b10bcab338b362b909491fea1cf13de66f5f17c0
          Author: Keith Laban <klaban1@bloomberg.net>
          Date: Wed Apr 5 14:57:28 2017 -0400

          SOLR-10047 - Updated javadoc/renamed class/added getReaderCacheHelper

          commit 90ecf5a4ae4feaf3efc42a1ed8643ad21e1c73ce
          Author: Keith Laban <klaban1@bloomberg.net>
          Date: Wed Jan 18 16:39:51 2017 -0500

          SOLR-10047 - SolrIndexSearcher, UninvertingReader, uninvert docvalues per segment

          (cherry picked from commit 4da901a0728239ac4e87b662533f966158991948)

          1. Conflicts:
          2. solr/core/src/test/org/apache/solr/schema/DocValuesTest.java
          Show
          jira-bot ASF subversion and git services added a comment - Commit 3e4629b8dfc6942fe391b741a8fb7d7baaeb854d in lucene-solr's branch refs/heads/branch_6x from Shalin Shekhar Mangar [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=3e4629b ] SOLR-10047 : Mismatched Docvalues segments cause exception in Sorting/Faceting. Solr now uninverts per segment to avoid such exceptions Squashed commit of the following: commit c38f4cabc2828ee83b53b931dd829e29a3e1701c Author: Keith Laban <kelaban17@gmail.com> Date: Tue Apr 11 17:17:05 2017 -0400 reverted tests to using old wrap interface commit 806f33e092491cc6a2ee292d2934c76171e40dc7 Author: Keith Laban <kelaban17@gmail.com> Date: Tue Apr 11 17:13:34 2017 -0400 updated UninvertingReader.wrap / tests commit b10bcab338b362b909491fea1cf13de66f5f17c0 Author: Keith Laban <klaban1@bloomberg.net> Date: Wed Apr 5 14:57:28 2017 -0400 SOLR-10047 - Updated javadoc/renamed class/added getReaderCacheHelper commit 90ecf5a4ae4feaf3efc42a1ed8643ad21e1c73ce Author: Keith Laban <klaban1@bloomberg.net> Date: Wed Jan 18 16:39:51 2017 -0500 SOLR-10047 - SolrIndexSearcher, UninvertingReader, uninvert docvalues per segment (cherry picked from commit 4da901a0728239ac4e87b662533f966158991948) Conflicts: solr/core/src/test/org/apache/solr/schema/DocValuesTest.java
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 3e4629b8dfc6942fe391b741a8fb7d7baaeb854d in lucene-solr's branch refs/heads/branch_6x from Shalin Shekhar Mangar
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=3e4629b ]

          SOLR-10047: Mismatched Docvalues segments cause exception in Sorting/Faceting. Solr now uninverts per segment to avoid such exceptions

          Squashed commit of the following:

          commit c38f4cabc2828ee83b53b931dd829e29a3e1701c
          Author: Keith Laban <kelaban17@gmail.com>
          Date: Tue Apr 11 17:17:05 2017 -0400

          reverted tests to using old wrap interface

          commit 806f33e092491cc6a2ee292d2934c76171e40dc7
          Author: Keith Laban <kelaban17@gmail.com>
          Date: Tue Apr 11 17:13:34 2017 -0400

          updated UninvertingReader.wrap / tests

          commit b10bcab338b362b909491fea1cf13de66f5f17c0
          Author: Keith Laban <klaban1@bloomberg.net>
          Date: Wed Apr 5 14:57:28 2017 -0400

          SOLR-10047 - Updated javadoc/renamed class/added getReaderCacheHelper

          commit 90ecf5a4ae4feaf3efc42a1ed8643ad21e1c73ce
          Author: Keith Laban <klaban1@bloomberg.net>
          Date: Wed Jan 18 16:39:51 2017 -0500

          SOLR-10047 - SolrIndexSearcher, UninvertingReader, uninvert docvalues per segment

          (cherry picked from commit 4da901a0728239ac4e87b662533f966158991948)

          1. Conflicts:
          2. solr/core/src/test/org/apache/solr/schema/DocValuesTest.java
          Show
          jira-bot ASF subversion and git services added a comment - Commit 3e4629b8dfc6942fe391b741a8fb7d7baaeb854d in lucene-solr's branch refs/heads/branch_6x from Shalin Shekhar Mangar [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=3e4629b ] SOLR-10047 : Mismatched Docvalues segments cause exception in Sorting/Faceting. Solr now uninverts per segment to avoid such exceptions Squashed commit of the following: commit c38f4cabc2828ee83b53b931dd829e29a3e1701c Author: Keith Laban <kelaban17@gmail.com> Date: Tue Apr 11 17:17:05 2017 -0400 reverted tests to using old wrap interface commit 806f33e092491cc6a2ee292d2934c76171e40dc7 Author: Keith Laban <kelaban17@gmail.com> Date: Tue Apr 11 17:13:34 2017 -0400 updated UninvertingReader.wrap / tests commit b10bcab338b362b909491fea1cf13de66f5f17c0 Author: Keith Laban <klaban1@bloomberg.net> Date: Wed Apr 5 14:57:28 2017 -0400 SOLR-10047 - Updated javadoc/renamed class/added getReaderCacheHelper commit 90ecf5a4ae4feaf3efc42a1ed8643ad21e1c73ce Author: Keith Laban <klaban1@bloomberg.net> Date: Wed Jan 18 16:39:51 2017 -0500 SOLR-10047 - SolrIndexSearcher, UninvertingReader, uninvert docvalues per segment (cherry picked from commit 4da901a0728239ac4e87b662533f966158991948) Conflicts: solr/core/src/test/org/apache/solr/schema/DocValuesTest.java
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 3e4629b8dfc6942fe391b741a8fb7d7baaeb854d in lucene-solr's branch refs/heads/branch_6x from Shalin Shekhar Mangar
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=3e4629b ]

          SOLR-10047: Mismatched Docvalues segments cause exception in Sorting/Faceting. Solr now uninverts per segment to avoid such exceptions

          Squashed commit of the following:

          commit c38f4cabc2828ee83b53b931dd829e29a3e1701c
          Author: Keith Laban <kelaban17@gmail.com>
          Date: Tue Apr 11 17:17:05 2017 -0400

          reverted tests to using old wrap interface

          commit 806f33e092491cc6a2ee292d2934c76171e40dc7
          Author: Keith Laban <kelaban17@gmail.com>
          Date: Tue Apr 11 17:13:34 2017 -0400

          updated UninvertingReader.wrap / tests

          commit b10bcab338b362b909491fea1cf13de66f5f17c0
          Author: Keith Laban <klaban1@bloomberg.net>
          Date: Wed Apr 5 14:57:28 2017 -0400

          SOLR-10047 - Updated javadoc/renamed class/added getReaderCacheHelper

          commit 90ecf5a4ae4feaf3efc42a1ed8643ad21e1c73ce
          Author: Keith Laban <klaban1@bloomberg.net>
          Date: Wed Jan 18 16:39:51 2017 -0500

          SOLR-10047 - SolrIndexSearcher, UninvertingReader, uninvert docvalues per segment

          (cherry picked from commit 4da901a0728239ac4e87b662533f966158991948)

          1. Conflicts:
          2. solr/core/src/test/org/apache/solr/schema/DocValuesTest.java
          Show
          jira-bot ASF subversion and git services added a comment - Commit 3e4629b8dfc6942fe391b741a8fb7d7baaeb854d in lucene-solr's branch refs/heads/branch_6x from Shalin Shekhar Mangar [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=3e4629b ] SOLR-10047 : Mismatched Docvalues segments cause exception in Sorting/Faceting. Solr now uninverts per segment to avoid such exceptions Squashed commit of the following: commit c38f4cabc2828ee83b53b931dd829e29a3e1701c Author: Keith Laban <kelaban17@gmail.com> Date: Tue Apr 11 17:17:05 2017 -0400 reverted tests to using old wrap interface commit 806f33e092491cc6a2ee292d2934c76171e40dc7 Author: Keith Laban <kelaban17@gmail.com> Date: Tue Apr 11 17:13:34 2017 -0400 updated UninvertingReader.wrap / tests commit b10bcab338b362b909491fea1cf13de66f5f17c0 Author: Keith Laban <klaban1@bloomberg.net> Date: Wed Apr 5 14:57:28 2017 -0400 SOLR-10047 - Updated javadoc/renamed class/added getReaderCacheHelper commit 90ecf5a4ae4feaf3efc42a1ed8643ad21e1c73ce Author: Keith Laban <klaban1@bloomberg.net> Date: Wed Jan 18 16:39:51 2017 -0500 SOLR-10047 - SolrIndexSearcher, UninvertingReader, uninvert docvalues per segment (cherry picked from commit 4da901a0728239ac4e87b662533f966158991948) Conflicts: solr/core/src/test/org/apache/solr/schema/DocValuesTest.java
          Hide
          shalinmangar Shalin Shekhar Mangar added a comment -

          Thanks Keith!

          I forgot to add the magic line to the commit which auto-closes the PR so you have to do that manually, sorry!

          Show
          shalinmangar Shalin Shekhar Mangar added a comment - Thanks Keith! I forgot to add the magic line to the commit which auto-closes the PR so you have to do that manually, sorry!
          Hide
          steve_rowe Steve Rowe added a comment -

          Policeman Jenkins failure reproduces for me https://jenkins.thetaphi.de/job/Lucene-Solr-master-Linux/19429/:

          Checking out Revision 48d54ac45860a1b75bfd79aaffe9d4d24c2ad5a8 (refs/remotes/origin/master)
          [...]
             [junit4]   2> NOTE: reproduce with: ant test  -Dtestcase=DocValuesTest -Dtests.method=testHalfAndHalfDocValues -Dtests.seed=D4C321F7B9F8C0F3 -Dtests.multiplier=3 -Dtests.slow=true -Dtests.locale=hu-HU -Dtests.timezone=WET -Dtests.asserts=true -Dtests.file.encoding=UTF-8
             [junit4] FAILURE 0.04s J1 | DocValuesTest.testHalfAndHalfDocValues <<<
             [junit4]    > Throwable #1: java.lang.AssertionError: expected:<3> but was:<2>
             [junit4]    > 	at __randomizedtesting.SeedInfo.seed([D4C321F7B9F8C0F3:1B0859436D75536E]:0)
             [junit4]    > 	at org.apache.solr.schema.DocValuesTest.testHalfAndHalfDocValues(DocValuesTest.java:192)
          [...]
             [junit4]   2> NOTE: test params are: codec=Asserting(Lucene70), sim=RandomSimilarity(queryNorm=false): {}, locale=hu-HU, timezone=WET
             [junit4]   2> NOTE: Linux 4.4.0-72-generic i386/Oracle Corporation 1.8.0_121 (32-bit)/cpus=12,threads=1,free=146280496,total=518979584
          

          When I run the repro line on my Linux box, the actual value was 1 instead of the 2 though, so at a minimum there's some flakiness here:

          [junit4] > Throwable #1: java.lang.AssertionError: expected:<3> but was:<1>

          Show
          steve_rowe Steve Rowe added a comment - Policeman Jenkins failure reproduces for me https://jenkins.thetaphi.de/job/Lucene-Solr-master-Linux/19429/ : Checking out Revision 48d54ac45860a1b75bfd79aaffe9d4d24c2ad5a8 (refs/remotes/origin/master) [...] [junit4] 2> NOTE: reproduce with: ant test -Dtestcase=DocValuesTest -Dtests.method=testHalfAndHalfDocValues -Dtests.seed=D4C321F7B9F8C0F3 -Dtests.multiplier=3 -Dtests.slow=true -Dtests.locale=hu-HU -Dtests.timezone=WET -Dtests.asserts=true -Dtests.file.encoding=UTF-8 [junit4] FAILURE 0.04s J1 | DocValuesTest.testHalfAndHalfDocValues <<< [junit4] > Throwable #1: java.lang.AssertionError: expected:<3> but was:<2> [junit4] > at __randomizedtesting.SeedInfo.seed([D4C321F7B9F8C0F3:1B0859436D75536E]:0) [junit4] > at org.apache.solr.schema.DocValuesTest.testHalfAndHalfDocValues(DocValuesTest.java:192) [...] [junit4] 2> NOTE: test params are: codec=Asserting(Lucene70), sim=RandomSimilarity(queryNorm=false): {}, locale=hu-HU, timezone=WET [junit4] 2> NOTE: Linux 4.4.0-72-generic i386/Oracle Corporation 1.8.0_121 (32-bit)/cpus=12,threads=1,free=146280496,total=518979584 When I run the repro line on my Linux box, the actual value was 1 instead of the 2 though, so at a minimum there's some flakiness here: [junit4] > Throwable #1: java.lang.AssertionError: expected:<3> but was:<1>
          Hide
          k317h Keith Laban added a comment -

          Thanks Steve, I am able to reproduce this using this seed. The test commits three documents and relies on them not being merged into a single segment to verify the behavior of this patch works correctly. It seems like with this seed the merge policy is merging these three segments.

          What is the best way to setup this test to assure that no merges occur?

          Show
          k317h Keith Laban added a comment - Thanks Steve, I am able to reproduce this using this seed. The test commits three documents and relies on them not being merged into a single segment to verify the behavior of this patch works correctly. It seems like with this seed the merge policy is merging these three segments. What is the best way to setup this test to assure that no merges occur?
          Hide
          hossman Hoss Man added a comment -

          MergePolicy settings are normally randomized by the solr test configs.

          If a test is dependent on controlling exact when a merge does/doesn't happen, you'll need to force the mergepolicy using something like NoMergePolicyFactory

          Example from TestInPlaceUpdatesDistrib...

          // we need consistent segments that aren't re-ordered on merge because we're
          // asserting inplace updates happen by checking the internal [docid]
          systemSetPropertySolrTestsMergePolicyFactory(NoMergePolicyFactory.class.getName());
          
          // HACK: Don't use a RandomMergePolicy, but only use the mergePolicyFactory that we've just set
          System.setProperty(SYSTEM_PROPERTY_SOLR_TESTS_USEMERGEPOLICYFACTORY, "true");
          System.setProperty(SYSTEM_PROPERTY_SOLR_TESTS_USEMERGEPOLICY, "false");
          

          (the last two lines are only needed because we're phasing out MergePolicy for MergePolicyFactory)

          Show
          hossman Hoss Man added a comment - MergePolicy settings are normally randomized by the solr test configs. If a test is dependent on controlling exact when a merge does/doesn't happen, you'll need to force the mergepolicy using something like NoMergePolicyFactory Example from TestInPlaceUpdatesDistrib... // we need consistent segments that aren't re-ordered on merge because we're // asserting inplace updates happen by checking the internal [docid] systemSetPropertySolrTestsMergePolicyFactory(NoMergePolicyFactory.class.getName()); // HACK: Don't use a RandomMergePolicy, but only use the mergePolicyFactory that we've just set System .setProperty(SYSTEM_PROPERTY_SOLR_TESTS_USEMERGEPOLICYFACTORY, " true " ); System .setProperty(SYSTEM_PROPERTY_SOLR_TESTS_USEMERGEPOLICY, " false " ); (the last two lines are only needed because we're phasing out MergePolicy for MergePolicyFactory)
          Hide
          shalinmangar Shalin Shekhar Mangar added a comment -

          Re-opening due to test failures. I'll put up a patch with the fix suggested by Hoss.

          Show
          shalinmangar Shalin Shekhar Mangar added a comment - Re-opening due to test failures. I'll put up a patch with the fix suggested by Hoss.
          Hide
          k317h Keith Laban added a comment -

          Thanks Hoss, Shalin Shekhar Mangar – no need, I will take a look at it today.

          Show
          k317h Keith Laban added a comment - Thanks Hoss, Shalin Shekhar Mangar – no need, I will take a look at it today.
          Hide
          shalinmangar Shalin Shekhar Mangar added a comment -

          Thanks Keith but I have already prepared the patch. I'm just waiting for the results of a round of beasting before I commit.

          Attached patch moves the new test into its own class so that we can use NoMergePolicy.

          Show
          shalinmangar Shalin Shekhar Mangar added a comment - Thanks Keith but I have already prepared the patch. I'm just waiting for the results of a round of beasting before I commit. Attached patch moves the new test into its own class so that we can use NoMergePolicy.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user kelaban closed the pull request at:

          https://github.com/apache/lucene-solr/pull/145

          Show
          githubbot ASF GitHub Bot added a comment - Github user kelaban closed the pull request at: https://github.com/apache/lucene-solr/pull/145
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user kelaban opened a pull request:

          https://github.com/apache/lucene-solr/pull/195

          SOLR-10047 - Fix test: Use NoMergePolicy in testing

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/kelaban/lucene-solr jira/master/SOLR-10047/fix-tests-uninverting-reader-per-semnet

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/lucene-solr/pull/195.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #195



          Show
          githubbot ASF GitHub Bot added a comment - GitHub user kelaban opened a pull request: https://github.com/apache/lucene-solr/pull/195 SOLR-10047 - Fix test: Use NoMergePolicy in testing You can merge this pull request into a Git repository by running: $ git pull https://github.com/kelaban/lucene-solr jira/master/ SOLR-10047 /fix-tests-uninverting-reader-per-semnet Alternatively you can review and apply these changes as the patch at: https://github.com/apache/lucene-solr/pull/195.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #195
          Hide
          k317h Keith Laban added a comment -

          Funny, I did the same thing. I closed the old PR and put in a new one – https://github.com/apache/lucene-solr/pull/195/files . This can be closed if you find your patch to be satisfactory.

          Show
          k317h Keith Laban added a comment - Funny, I did the same thing. I closed the old PR and put in a new one – https://github.com/apache/lucene-solr/pull/195/files . This can be closed if you find your patch to be satisfactory.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user chatman commented on a diff in the pull request:

          https://github.com/apache/lucene-solr/pull/195#discussion_r113005049

          — Diff: solr/core/src/test/org/apache/solr/schema/DocValuesEnabledLaterTest.java —
          @@ -0,0 +1,140 @@
          +/*
          + * Licensed to the Apache Software Foundation (ASF) under one or more
          + * contributor license agreements. See the NOTICE file distributed with
          + * this work for additional information regarding copyright ownership.
          + * The ASF licenses this file to You under the Apache License, Version 2.0
          + * (the "License"); you may not use this file except in compliance with
          + * the License. You may obtain a copy of the License at
          + *
          + * http://www.apache.org/licenses/LICENSE-2.0
          + *
          + * Unless required by applicable law or agreed to in writing, software
          + * distributed under the License is distributed on an "AS IS" BASIS,
          + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + * See the License for the specific language governing permissions and
          + * limitations under the License.
          + */
          +package org.apache.solr.schema;
          +
          +import java.lang.invoke.MethodHandles;
          +
          +import org.apache.lucene.document.Document;
          +import org.apache.lucene.index.DirectoryReader;
          +import org.apache.lucene.index.DocValuesType;
          +import org.apache.lucene.index.FieldInfos;
          +import org.apache.lucene.index.LeafReader;
          +import org.apache.lucene.index.LeafReaderContext;
          +import org.apache.lucene.index.MultiFields;
          +import org.apache.solr.SolrTestCaseJ4;
          +import org.apache.solr.core.SolrCore;
          +import org.apache.solr.index.NoMergePolicyFactory;
          +import org.apache.solr.search.SolrIndexSearcher;
          +import org.apache.solr.util.RefCounted;
          +import org.junit.BeforeClass;
          +import org.junit.Test;
          +import org.slf4j.Logger;
          +import org.slf4j.LoggerFactory;
          +
          +public class DocValuesEnabledLaterTest extends SolrTestCaseJ4 {
          +
          + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
          +
          + @BeforeClass
          + public static void beforeTests() throws Exception {
          + initCore("solrconfig-basic.xml", "schema-docValues.xml");
          +
          + // we need consistent segments that aren't re-ordered on merge because we're
          + // asserting inplace updates happen by checking the internal [docid]
          — End diff –

          Lets remove references to inplace updates here.

          Show
          githubbot ASF GitHub Bot added a comment - Github user chatman commented on a diff in the pull request: https://github.com/apache/lucene-solr/pull/195#discussion_r113005049 — Diff: solr/core/src/test/org/apache/solr/schema/DocValuesEnabledLaterTest.java — @@ -0,0 +1,140 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.schema; + +import java.lang.invoke.MethodHandles; + +import org.apache.lucene.document.Document; +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.DocValuesType; +import org.apache.lucene.index.FieldInfos; +import org.apache.lucene.index.LeafReader; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.MultiFields; +import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.core.SolrCore; +import org.apache.solr.index.NoMergePolicyFactory; +import org.apache.solr.search.SolrIndexSearcher; +import org.apache.solr.util.RefCounted; +import org.junit.BeforeClass; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class DocValuesEnabledLaterTest extends SolrTestCaseJ4 { + + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + @BeforeClass + public static void beforeTests() throws Exception { + initCore("solrconfig-basic.xml", "schema-docValues.xml"); + + // we need consistent segments that aren't re-ordered on merge because we're + // asserting inplace updates happen by checking the internal [docid] — End diff – Lets remove references to inplace updates here.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 56e1ad484a2c7431932a95e442bc1a584f60698e in lucene-solr's branch refs/heads/master from Shalin Shekhar Mangar
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=56e1ad4 ]

          SOLR-10047: Move test into its own test class and force use of NoMergePolicy to fix test failures

          This closes #195

          Show
          jira-bot ASF subversion and git services added a comment - Commit 56e1ad484a2c7431932a95e442bc1a584f60698e in lucene-solr's branch refs/heads/master from Shalin Shekhar Mangar [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=56e1ad4 ] SOLR-10047 : Move test into its own test class and force use of NoMergePolicy to fix test failures This closes #195
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/lucene-solr/pull/195

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/lucene-solr/pull/195
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit ef56f3868834243071b0d663aaab19fa21212495 in lucene-solr's branch refs/heads/branch_6x from Shalin Shekhar Mangar
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=ef56f38 ]

          SOLR-10047: Move test into its own test class and force use of NoMergePolicy to fix test failures

          This closes #195

          (cherry picked from commit 56e1ad4)

          Show
          jira-bot ASF subversion and git services added a comment - Commit ef56f3868834243071b0d663aaab19fa21212495 in lucene-solr's branch refs/heads/branch_6x from Shalin Shekhar Mangar [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=ef56f38 ] SOLR-10047 : Move test into its own test class and force use of NoMergePolicy to fix test failures This closes #195 (cherry picked from commit 56e1ad4)

            People

            • Assignee:
              shalinmangar Shalin Shekhar Mangar
              Reporter:
              k317h Keith Laban
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development