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

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?

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.

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

Nice!

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.

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

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!

No problem. I'll look into this tomorrow.

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.

Committed to trunk and 3x branch.