Solr
  1. Solr
  2. SOLR-3196

partialResults response header not propagated in distributed search

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.5, 4.0-ALPHA
    • Fix Version/s: 3.6, 4.0-ALPHA
    • Component/s: search
    • Labels:

      Description

      For timeAllowed=true requests, the response contains a partialResults header that indicates when a search was terminated early due to running out of time. This header is being discarded by the collator. Patch to follow.

      1. SOLR-3196-3x.patch
        1 kB
        Erick Erickson
      2. SOLR-3196-partialResults-header.patch
        1 kB
        Russell Black

        Activity

        Hide
        Mark Miller added a comment -

        Nice, thanks guys. We should prob add a simple test too - I can probably take this issue on if no one else jumps on it.

        Show
        Mark Miller added a comment - Nice, thanks guys. We should prob add a simple test too - I can probably take this issue on if no one else jumps on it.
        Hide
        Russell Black added a comment -

        I looked into writing a test case for this, but ran into some difficulties. The problem is that for the small data sets used by the test cases, all queries return within 1 millisecond, so I can't force the timeout. (1 ms is the lowest value supported for the timeAllowed parameter, passing 0 is the same as not passing a timeout). I haven't been able to come up with a query that takes sufficiently long to test timeAllowed. It appears that there are no test cases that test for the presence of the partialResults header even for non-distributed searches. I am confident that this patch works, however, since I have tested it thoroughly in our own index. It's a pretty uncomplicated patch.

        Show
        Russell Black added a comment - I looked into writing a test case for this, but ran into some difficulties. The problem is that for the small data sets used by the test cases, all queries return within 1 millisecond, so I can't force the timeout. (1 ms is the lowest value supported for the timeAllowed parameter, passing 0 is the same as not passing a timeout). I haven't been able to come up with a query that takes sufficiently long to test timeAllowed . It appears that there are no test cases that test for the presence of the partialResults header even for non-distributed searches. I am confident that this patch works, however, since I have tested it thoroughly in our own index. It's a pretty uncomplicated patch.
        Hide
        Russell Black added a comment -

        Will it be possible to have this committed to 3x as well? The patch should apply just fine to the 3x branch and we'd love to have this make it into 3.6.

        Show
        Russell Black added a comment - Will it be possible to have this committed to 3x as well? The patch should apply just fine to the 3x branch and we'd love to have this make it into 3.6.
        Hide
        Erick Erickson added a comment -

        Patch didn't apply to 3x, apparently a few things moved around.

        Russel:
        Could you take a quick check and see if this looks OK for 3x?

        Any back-compat issues with changing what comes back in the responseHeader?

        Show
        Erick Erickson added a comment - Patch didn't apply to 3x, apparently a few things moved around. Russel: Could you take a quick check and see if this looks OK for 3x? Any back-compat issues with changing what comes back in the responseHeader?
        Hide
        Russell Black added a comment -

        Erick,

        Thanks for trying, i'll take a look at what's going on.

        Show
        Russell Black added a comment - Erick, Thanks for trying, i'll take a look at what's going on.
        Hide
        Russell Black added a comment -

        I'm not sure why the original patch fails to apply to 3x. However, your patch looks good to me.

        I don't see any problems with backwards compatibility. This patch has the effect of adding a partialResults true header, making it behave the same as a non-distributed request for time limited queries. It's hard for me to imagine a situation where this would break backwards-compatibility.

        Show
        Russell Black added a comment - I'm not sure why the original patch fails to apply to 3x. However, your patch looks good to me. I don't see any problems with backwards compatibility. This patch has the effect of adding a partialResults true header, making it behave the same as a non-distributed request for time limited queries. It's hard for me to imagine a situation where this would break backwards-compatibility.
        Hide
        Erick Erickson added a comment -

        4.x r: 1301097
        3.6 r: 1301096

        Right, after looking a bit more closely, there's nothing here that would break back-compat, that was just my paranoia was at work.

        Show
        Erick Erickson added a comment - 4.x r: 1301097 3.6 r: 1301096 Right, after looking a bit more closely, there's nothing here that would break back-compat, that was just my paranoia was at work.

          People

          • Assignee:
            Unassigned
            Reporter:
            Russell Black
          • Votes:
            2 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development