Solr
  1. Solr
  2. SOLR-3195

timeAllowed is ignored for grouping queries

    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 grouping queries, the timeAllowed parameter is ignored. I couldn't find this limitation documented anywhere, and we would like to use that feature in our production system. I have created a patch that makes grouping queries respect the timeAllowed parameter. The test cases all pass, and it and it seems to work well. I added support for both distributed and non-distributed grouping.

        Activity

        Hide
        Martijn van Groningen added a comment -

        This was never implemented. Good catch! The patch looks fine. I'll commit soon.

        Show
        Martijn van Groningen added a comment - This was never implemented. Good catch! The patch looks fine. I'll commit soon.
        Hide
        Russell Black added a comment -

        Martijn,

        There is one thing I'd like your opinion on – for distributed grouping the first phase and second phase are given the full time allotment with my patch, essentially resulting in doubling the effect of the timeAllowed parameter. Do you think we should try to be smarter about it? We could subtract the time time of the first query from the timeAllowed parameter that gets handed to the second query, resulting in more accurate overall time.

        What do you think?

        Show
        Russell Black added a comment - Martijn, There is one thing I'd like your opinion on – for distributed grouping the first phase and second phase are given the full time allotment with my patch, essentially resulting in doubling the effect of the timeAllowed parameter. Do you think we should try to be smarter about it? We could subtract the time time of the first query from the timeAllowed parameter that gets handed to the second query, resulting in more accurate overall time. What do you think?
        Hide
        Martijn van Groningen added a comment -

        Russell, that is a good point. I think we just can use the qtime from the first query and subtract that from the specified timeAllowed. The qtime can be retrieved from the ShardResponse with the highest qtime.

        Show
        Martijn van Groningen added a comment - Russell, that is a good point. I think we just can use the qtime from the first query and subtract that from the specified timeAllowed. The qtime can be retrieved from the ShardResponse with the highest qtime.
        Hide
        Russell Black added a comment -

        Martijn, I have a patch with just such an implementation. I'll upload it in a few minutes.

        Show
        Russell Black added a comment - Martijn, I have a patch with just such an implementation. I'll upload it in a few minutes.
        Hide
        Martijn van Groningen added a comment -

        Nice!

        Show
        Martijn van Groningen added a comment - Nice!
        Hide
        Russell Black added a comment -

        OK, this patch subtracts time for the second phase. The one thing I did differently from your suggestion was to use ShardResponse.getSolrResponse().getElapsedTime() instead of QTime, since it is a more accurate measurement of total time spent on the first phase. If you want to use QTime, feel free to change it.

        Show
        Russell Black added a comment - OK, this patch subtracts time for the second phase. The one thing I did differently from your suggestion was to use ShardResponse.getSolrResponse().getElapsedTime() instead of QTime, since it is a more accurate measurement of total time spent on the first phase. If you want to use QTime, feel free to change it.
        Hide
        Russell Black added a comment -

        Also, would it be possible to get my backported patch into 3x?

        Show
        Russell Black added a comment - Also, would it be possible to get my backported patch into 3x?
        Hide
        Russell Black added a comment -

        I just realized that my patches also contain the patch I submitted for SOLR-3196 which is actually related to this one. Sorry of this creates confusion for you. It's the changes to QueryComponent.java that fix SOLR-3196. Thanks!

        Show
        Russell Black added a comment - I just realized that my patches also contain the patch I submitted for SOLR-3196 which is actually related to this one. Sorry of this creates confusion for you. It's the changes to QueryComponent.java that fix SOLR-3196 . Thanks!
        Hide
        Martijn van Groningen added a comment -

        No problem. I'll look into this tomorrow.

        Show
        Martijn van Groningen added a comment - No problem. I'll look into this tomorrow.
        Hide
        Martijn van Groningen added a comment -

        The difference between the elapsed time and qtime is that the first also takes into account the time spend on the trip from and to the shard where the shard request is executed. I think it is fine to include also this time in the time allowed for a query.

        Show
        Martijn van Groningen added a comment - The difference between the elapsed time and qtime is that the first also takes into account the time spend on the trip from and to the shard where the shard request is executed. I think it is fine to include also this time in the time allowed for a query.
        Hide
        Martijn van Groningen added a comment -

        Committed to trunk and 3x branch.

        Show
        Martijn van Groningen added a comment - Committed to trunk and 3x branch.

          People

          • Assignee:
            Martijn van Groningen
            Reporter:
            Russell Black
          • Votes:
            2 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development