prefetch for engagements all page

[valentijnscholten]

After updating to the latest dev I noticed the engagements_all page was slow, executing 300+ queries.
This PR adds some needed prefetching. Not sure if this removed at some point, but seems to work fine now.

[valentijnscholten]

wdyt @danielnaab

[danielnaab]

@valentijnscholten sorry to hear about the problems - I tested this locally on a copy of our prod db. I had found that while those prefetches reduce the query count, they nonetheless increase the total time to render the page. But this could definitely be due to something specific about our database. This is what I'm seeing (via django-debug-toolbar):

Before prefetches:

• Total CPU time: 4.8 sec
• 49 queries / 3.1 sec

With prefetches:

• Total CPU time: 21.8 sec
• 17 queries / 10.9 sec

So despite reducing the query count, the total time is ~4.5 times slower. This is with 12 rows in the rendered /engagements_all table, and no tags on any of the engagements. How does your total render time compare?

[valentijnscholten]

To be sure, are you looking at the page at /engagements_all ?
You're numbers look extremely slow for that small amount of engagements.

My numbers are:

mysql 488 engagements (143 products)

with prefetch:
900ms cpu / 28queries(70ms)

without
8000ms cpu / 330 queries (327ms)

postgres 3661 engagement (45 products)

with
6979ms cpu / 23 querie (59ms)

without
61000ms cpu / 2973 queries (2957ms)

When disabling the debug toolbar the without scenario becomes a bit faster, but still slower than without prefetching.

[valentijnscholten]

to be honest, that whole page should be rewritten because it does everything based on product instead of engagement.
so the paging doesn't work as expected and can render 100s of rows on a single page.

[danielnaab]

Yeah it's odd. We definitely found the queries themselves weren't the main issue with our performance, but something in the ORM layer. We do have a lot of tests, so I suspect the ORM-layer is getting caught up trying to map all the prefetched objects' over all the various relations.

For an idea, this shows our test count per engagement:

[valentijnscholten]

Yes, with those amounts of tests it will go crazy. For we should prefetch the count by using annotate instead of doing e.test_set.count, after which we don't need to prefetch the test_set anymore. But I don't know how to do that when the main query is for products, so again we would need to make the main query based on engagements.

[valentijnscholten]

OK, couldn't help myself and updated the PR. It now counts the tests using an SQL query instead of loading all tests into memory. Could you try this maybe?

[danielnaab]

@valentijnscholten that works great - here's updated numbers:

Before prefetches:

• Total CPU time: 4.8 sec
• 49 queries / 3.1 sec

With prefetches

• Total CPU time: 21.8 sec
• 17 queries / 10.9 sec

With prefetches + prefetch count:

• Total CPU time: 4.8 sec
• 33 queries / 2.1 sec

[valentijnscholten]

Not bad with that CRAZY amount of tests 😜

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved. A maintainer will review the pull request shortly.