Lucene - Core
  1. Lucene - Core
  2. LUCENE-4938

IndexSearcher.search() with sort doesnt do min(maxdoc, n)

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.3.1, 6.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      It does this without a sort though.

      This caused TestFunctionQuerySort.testSearchAfterWhenSortingByFunctionValues to OOM (why only sometimes?)

      1. LUCENE-4938.patch
        8 kB
        Robert Muir
      2. LUCENE-4938.patch
        7 kB
        Robert Muir
      3. LUCENE-4938.patch
        2 kB
        Robert Muir

        Activity

        Hide
        Robert Muir added a comment -

        This is the patch i sent to the dev list. But we should add simple tests for this (also for the other search method).

        Also i want to know how the test ever passed... is this due to something AssertingIndexSearcher is doing?

        Show
        Robert Muir added a comment - This is the patch i sent to the dev list. But we should add simple tests for this (also for the other search method). Also i want to know how the test ever passed... is this due to something AssertingIndexSearcher is doing?
        Hide
        Michael McCandless added a comment -

        I think the issue happens only when newSearcher passes an ExecutorService? I wired LTC.newSearcher temporarily to always pass that and it looks like it's always failing ...

        Show
        Michael McCandless added a comment - I think the issue happens only when newSearcher passes an ExecutorService? I wired LTC.newSearcher temporarily to always pass that and it looks like it's always failing ...
        Hide
        Uwe Schindler added a comment -

        Yes!

        I checked the code, this is what happens. The problem is in IndexSearcher that it does not everywhere uses the Math.min() logic, in some code paths it is missing.

        Show
        Uwe Schindler added a comment - Yes! I checked the code, this is what happens. The problem is in IndexSearcher that it does not everywhere uses the Math.min() logic, in some code paths it is missing.
        Hide
        Robert Muir added a comment -

        Thanks for investigating. I was just not happy fixing this bug if there is another bug.

        In my opinion all thats needed here now is simple tests (for core lucene) that tests with Integer.MAX_VALUE, so that this is not tested in a confusing roundabout way.

        Show
        Robert Muir added a comment - Thanks for investigating. I was just not happy fixing this bug if there is another bug. In my opinion all thats needed here now is simple tests (for core lucene) that tests with Integer.MAX_VALUE, so that this is not tested in a confusing roundabout way.
        Hide
        Robert Muir added a comment -

        Updated patch, i added a test to try to test all indexsearcher methods for this.

        it currently fails.

        Show
        Robert Muir added a comment - Updated patch, i added a test to try to test all indexsearcher methods for this. it currently fails.
        Hide
        Robert Muir added a comment -

        oh it was just one missing place: fixed here.

        Test passes now.

        Show
        Robert Muir added a comment - oh it was just one missing place: fixed here. Test passes now.
        Hide
        Commit Tag Bot added a comment -

        [trunk commit] rmuir
        http://svn.apache.org/viewvc?view=revision&revision=1469889

        LUCENE-4938: IndexSearcher.search() with sort doesnt do min(maxdoc, n)

        Show
        Commit Tag Bot added a comment - [trunk commit] rmuir http://svn.apache.org/viewvc?view=revision&revision=1469889 LUCENE-4938 : IndexSearcher.search() with sort doesnt do min(maxdoc, n)
        Hide
        Commit Tag Bot added a comment -

        [branch_4x commit] rmuir
        http://svn.apache.org/viewvc?view=revision&revision=1469902

        LUCENE-4938: IndexSearcher.search() with sort doesnt do min(maxdoc, n)

        Show
        Commit Tag Bot added a comment - [branch_4x commit] rmuir http://svn.apache.org/viewvc?view=revision&revision=1469902 LUCENE-4938 : IndexSearcher.search() with sort doesnt do min(maxdoc, n)
        Hide
        Robert Muir added a comment -

        Thanks Uwe and Mike for detective-work!

        Show
        Robert Muir added a comment - Thanks Uwe and Mike for detective-work!
        Hide
        Steve Rowe added a comment -

        If there are no objections, I'd like to backport this to 4.3.1.

        Show
        Steve Rowe added a comment - If there are no objections, I'd like to backport this to 4.3.1.
        Hide
        Shalin Shekhar Mangar added a comment -

        Back ported to 4.3.1 r1483384.

        Show
        Shalin Shekhar Mangar added a comment - Back ported to 4.3.1 r1483384.
        Hide
        Shalin Shekhar Mangar added a comment -

        Bulk closing after 4.3.1 release

        Show
        Shalin Shekhar Mangar added a comment - Bulk closing after 4.3.1 release

          People

          • Assignee:
            Unassigned
            Reporter:
            Robert Muir
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development