Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance improvement of the outgoing products page when editing OCs #5822

Merged
merged 1 commit into from Jul 31, 2020

Conversation

luisramos0
Copy link
Contributor

@luisramos0 luisramos0 commented Jul 24, 2020

What? Why?

Testing #5775 in production we realized loading the enterprise is taking a very long time (15secs for the enterprise experiencing the bug, it's an OC with 400+ products coming from 20+ suppliers). I looked at the code I realized I could improve performance a little. See commit message.

This improvement will mitigate the problem in #5813, but wont solve it.

What should we test?

Like in #5775 verify the OC page, specifically outgoing exchanges, is still working with the option "coordinator inventory only". It should just be faster, I wonder if that can be seen before and after the PR :-)

Release notes

Changelog Category: Changed
Performance improvement of the outgoing products page when editing OCs with large numbers of products.

@luisramos0 luisramos0 self-assigned this Jul 24, 2020
@luisramos0 luisramos0 changed the title Remove select from relation Performance improvement of the outgoing products page when editing OCs Jul 24, 2020
@filipefurtad0
Copy link
Contributor

Hey @luisramos0 ,

Thank you for this fix!
But it looks like this PR has not been pushed to Semaphore yet, I find no link to stage this. Could you please have a look? Thanks.

@sauloperez
Copy link
Contributor

for some reason, it refused to run the build. You might need to push force @luisramos0 . Manually retrying fails too

Screenshot from 2020-07-31 09-15-09

This relation is only used above for a call to empty? so we don't need to worry about the select part of the query, specially not introducing an expensive DISTINCT
@luisramos0
Copy link
Contributor Author

ok, force pushed, the build is there now 👍

@filipefurtad0 filipefurtad0 self-assigned this Jul 31, 2020
@filipefurtad0 filipefurtad0 added the pr-staged-uk staging.openfoodnetwork.org.uk label Jul 31, 2020
@filipefurtad0
Copy link
Contributor

Hi @luisramos0,

I checked if the option "coordinator inventory only" is still working and tried to chase down faster page loads, for an OC with ~300 variants (id=203888). I had a few runs, on pressing "Expand All" and "Load All Variants". Also had a look at one with around ~700 variants (id=202632).

Page loads did appear slightly faster, and I did not observe any issues related to #5813 .

Looks good to go - thanks for improving this.

@filipefurtad0 filipefurtad0 removed the pr-staged-uk staging.openfoodnetwork.org.uk label Jul 31, 2020
@luisramos0 luisramos0 merged commit b025d5a into openfoodfoundation:master Jul 31, 2020
@luisramos0 luisramos0 deleted the improve_ocs_perf branch July 31, 2020 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants