Code review comment for lp:~benji/launchpad/bug-781600

Revision history for this message
Robert Collins (lifeless) wrote :

On Thu, Jul 14, 2011 at 8:48 AM, Benji York <email address hidden> wrote:
>
>> It's kind of a shame that we have to materialize the whole results set here,
>> since for huge size, it will be batched anyway by the web service code. Would
>> it make sense to use an generator here? Maybe not, since we'd still retrieve
>> all results for later batches.
>
> Given that we use ORDER BY, I suspect the only savings would be not
> transmitting the non-batched results from the DB server to the app
> server.  However, making the method a generator might still be a win
> because even though they will likely come back to get the rest, that
> request may well hit a different app server.  (I don't think we do any
> sort of request affinity but would like to know if we do.)
>
> Later... Nope, that didn't work.  lazr.restful exploded because it
> couldn't adapt the generator to IFiniteSequence.  I would have expected
> returning a generator to work, but I can't find any other instances in
> the code base.  I've left it as a non-generator for the time being but
> would appreciate any knowledge that can be shared on the topic.

So, order by does not on its own mean that the whole result set is
materialised; in fact, if there is an index that can satisfy the
ordering, and its limited, pg seems to prioritise it very highly.

It looks like the grouping could be done in the DB ?
If so then returning the result set may make sense.

Another alternative is to use the new batchnavigator 1.2.5 facilities
to do batch memos rather than slicing. However that may require some
glue into the lazr stuff.. OTOH you're pretty well placed for that
sort of surgery :)

-Rob

« Back to merge proposal