Solr
  1. Solr
  2. SOLR-2949

QueryElevationComponent does not fully support distributed search

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.0-ALPHA
    • Component/s: None
    • Labels:
      None

      Description

      The QueryElevationComponent does not fully support distributed search. Add tests and make a fix for it.

      1. SOLR-2949.patch
        9 kB
        Mark Miller
      2. SOLR-2949.patch
        6 kB
        Mark Miller
      3. SOLR-2949-3.X.patch
        2 kB
        Patanachai Tangchaisin

        Issue Links

          Activity

          Grant Ingersoll created issue -
          Grant Ingersoll made changes -
          Field Original Value New Value
          Link This issue relates to SOLR-2580 [ SOLR-2580 ]
          Mark Miller made changes -
          Link This issue relates to SOLR-3252 [ SOLR-3252 ]
          Mark Miller made changes -
          Assignee Grant Ingersoll [ gsingers ] Mark Miller [ markrmiller@gmail.com ]
          Hide
          Mark Miller added a comment -

          A short term fix + a fix for SOLR-3252 + a test.

          Show
          Mark Miller added a comment - A short term fix + a fix for SOLR-3252 + a test.
          Mark Miller made changes -
          Attachment SOLR-2949.patch [ 12518625 ]
          Hide
          Yonik Seeley added a comment -

          In distributedProcess, I understand the switching on reverse=true for the QEC comparator, but it looks like it modifies all of the sort fields?
          AFAIK, it's only QEC that currently has a field comparator that is reverse of the natural ordering of the objects it creates.
          Perhaps we just need to check for instanceof ElevationComparatorSource?

          Show
          Yonik Seeley added a comment - In distributedProcess, I understand the switching on reverse=true for the QEC comparator, but it looks like it modifies all of the sort fields? AFAIK, it's only QEC that currently has a field comparator that is reverse of the natural ordering of the objects it creates. Perhaps we just need to check for instanceof ElevationComparatorSource?
          Hide
          Mark Miller added a comment -

          A better, more working patch.

          Show
          Mark Miller added a comment - A better, more working patch.
          Mark Miller made changes -
          Attachment SOLR-2949.patch [ 12518721 ]
          Hide
          Mark Miller added a comment -

          Perhaps we just need to check for instanceof ElevationComparatorSource?

          I had tried that, but it wasn't working - flipping them all helped make tests pass coincidently. A little more testing added and it fell down.

          Latest patch reverses the QEC comparator, uses reverse=true for the QEC sort fields and doesn't require the distributedProcess part.

          There is still a problem in the the QEC sort field used is 'id' - this causes a problem with distrib. If you change it to something like "elevate" that solves the distrib problem - but then some of the single node tests fail.

          For now its left at 'id' and sorting by id in distrib with QEC is not supported iwth this patch.

          Show
          Mark Miller added a comment - Perhaps we just need to check for instanceof ElevationComparatorSource? I had tried that, but it wasn't working - flipping them all helped make tests pass coincidently. A little more testing added and it fell down. Latest patch reverses the QEC comparator, uses reverse=true for the QEC sort fields and doesn't require the distributedProcess part. There is still a problem in the the QEC sort field used is 'id' - this causes a problem with distrib. If you change it to something like " elevate " that solves the distrib problem - but then some of the single node tests fail. For now its left at 'id' and sorting by id in distrib with QEC is not supported iwth this patch.
          Hide
          Mark Miller added a comment -

          Looks like sort by id doesnt work with QEC on a single instance either.

          Show
          Mark Miller added a comment - Looks like sort by id doesnt work with QEC on a single instance either.
          yonik committed 1302542 (1 file)
          Reviews: none

          SOLR-2949: distrib support for QEC

          Hide
          Yonik Seeley added a comment -

          Committed fix (the comparator was looking at _elevate_ and using that as the id field)

          Show
          Yonik Seeley added a comment - Committed fix (the comparator was looking at _elevate_ and using that as the id field)
          Yonik Seeley made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Fix Version/s 3.6 [ 12319065 ]
          Resolution Fixed [ 1 ]
          yonik committed 1302637 (1 file)
          yonik committed 1302661 (1 file)
          Hide
          Patanachai Tangchaisin added a comment -

          Could you port the fix to 3.6.x as well?

          It looks like changing reverse=true will fix this problem in 3.x truck without ZooKeeper stuffs.

          Attached is a Mark's patch without ZooKeeper code

          Show
          Patanachai Tangchaisin added a comment - Could you port the fix to 3.6.x as well? It looks like changing reverse=true will fix this problem in 3.x truck without ZooKeeper stuffs. Attached is a Mark's patch without ZooKeeper code
          Patanachai Tangchaisin made changes -
          Attachment SOLR-2949-3.X.patch [ 12525097 ]
          Hide
          Mark Miller added a comment -

          If we have another 3.x release - i half remember someone proposing we stop 3.x releases (other than bug fix).

          Show
          Mark Miller added a comment - If we have another 3.x release - i half remember someone proposing we stop 3.x releases (other than bug fix).
          Hide
          Mark Miller added a comment -

          Also please note: I think Yonik made an additional fix or two to my patch on his commit.

          Show
          Mark Miller added a comment - Also please note: I think Yonik made an additional fix or two to my patch on his commit.
          Uwe Schindler made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

            • Assignee:
              Mark Miller
              Reporter:
              Grant Ingersoll
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development