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

eager load records instead of n+1 for update_positions #3875

Merged

Conversation

BenMorganIO
Copy link
Contributor

Description

Improves the performance of update_positions by eager loading all of the records instead of 1-by-1.

Checklist:

Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, pushing this to the 3.1 milestone!

@kennyadsl kennyadsl added this to the 3.1.0 milestone Jan 8, 2021
@kennyadsl kennyadsl added release:major Breaking change on hold until next major release changelog:solidus_backend Changes to the solidus_backend gem labels Jan 8, 2021
@BenMorganIO
Copy link
Contributor Author

I mean, there are a lot of DB calls when updating positions, but at least there are now far fewer selects over many endpoints.

@kennyadsl
Copy link
Member

@BenMorganIO sure, the change is good. Solidus 3.0 is a major and should only contain the removal of deprecated code so it will be easier to upgrade for people that followed the instructions in logs. That's why I think this should go in 3.1, I hope this happens soon, will keep the community posted, thanks!

@kennyadsl kennyadsl merged commit 248b39f into solidusio:master Apr 30, 2021
@kennyadsl kennyadsl removed the release:major Breaking change on hold until next major release label Apr 30, 2021
@BenMorganIO BenMorganIO deleted the improve-update-positions-performance branch May 3, 2021 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:solidus_backend Changes to the solidus_backend gem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants