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

feat(medusa): Improve product update performances #3417

Merged
merged 26 commits into from
Mar 15, 2023

Conversation

adrien2p
Copy link
Member

@adrien2p adrien2p commented Mar 8, 2023

  • create 15 products during setup, each product has 50 variants with each a pricing
  • in each request, update the amount of each price for each variant of the product being updated

For simple cases where no props on the variants need to be updated or the prices amount did not changed, the update are avoided and therefore also improve by removing unnecessary steps in the flow.

A few methods have been migrated to bulk to avoid unnecessary queries and reduce the amount of those queries.

Other improvements have been added overall and you can look at the pr itself to get a better understanding.

NOTE
https:/medusajs/medusa-staging/pull/14

Test

// load testing config
export const scenarios = {
  scenarios: {
    updateProductVariantPrices: {
	  executor: 'ramping-arrival-rate',
	  startTime: '3s',
      startRate: 5,
      timeUnit: '1s', // we start at 5 iterations per second
	  stages: [
        { target: 15, duration: '5s' },
        { target: 15, duration: '5s' },
        { target: 0, duration: '5s' },
      ],
      preAllocatedVUs: 20,
      maxVUs: 50, // if the preAllocatedVUs are not enough, we can initialize more
      exec: "updateProductVariantPrices_"
    }
  }
};

The tests have been ran on a single instance on my local machine, the important thing to look at is the factor between
before/after more than the numbers themselves as it can change depending on the resources.

--

The data used are the same shape for both tests against master and against the pr

  • 15 products created
  • 50 variants per product
  • 1 price per variant
    Each request will update one product including all the variants and update the price of each variant, the data looks like this
// data to update in the post body
{
  variants: [
    // 49 other variants
    {
      id: ...,
      prices: [{
        currency_code: "usd",
        amount: 20000,
      }]
    }
  }]
}

You can see the results in the images with left (master) and right (the pr)

image (1)
image

RESOLVE CORE-1208

@vercel
Copy link

vercel bot commented Mar 8, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
medusa-docs ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 15, 2023 at 3:38PM (UTC)

@changeset-bot
Copy link

changeset-bot bot commented Mar 8, 2023

🦋 Changeset detected

Latest commit: 63102bf

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@adrien2p
Copy link
Member Author

@olivermrbl once bulk emit is merged, I ll be able to update that pr with it

expect(moneyAmountRepository.update).toHaveBeenCalledWith(
{ id: IdMap.getId("dkk") },
{
amount: 850,
Copy link
Member Author

Choose a reason for hiding this comment

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

self-review: Here only the amount changed, this is why the currecny_cde does not appear anymore

Copy link
Contributor

@olivermrbl olivermrbl left a comment

Choose a reason for hiding this comment

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

Couple of initial comments. Haven't gotten to the variant service changes yet :)

Copy link
Contributor

@olivermrbl olivermrbl left a comment

Choose a reason for hiding this comment

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

LGTM - this is a great piece of work @adrien2p!

nitpicking: Should we create types for some of these service method payloads? Think it is slightly messy with inline typings

packages/medusa/src/services/product-variant.ts Outdated Show resolved Hide resolved
@olivermrbl
Copy link
Contributor

@riqwan Will you give this a final review too? 🙏

@olivermrbl
Copy link
Contributor

@adrien2p Please add a changeset :)

Copy link
Contributor

@riqwan riqwan left a comment

Choose a reason for hiding this comment

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

This must've been a real test, looks great! 🎉

@adrien2p
Copy link
Member Author

LGTM - this is a great piece of work @adrien2p!

nitpicking: Should we create types for some of these service method payloads? Think it is slightly messy with inline typings

I ve includes the types @olivermrbl

@olivermrbl olivermrbl merged commit fa4049c into master Mar 15, 2023
@olivermrbl olivermrbl deleted the feat/product-update-perf branch March 15, 2023 16:05
olivermrbl added a commit that referenced this pull request Mar 31, 2023
* Fix issue on fixed total amount discount when using includes tax (#3472)

The calculation of the fixed discount amount breaks when having includes_tax setting active, due to the line item totals are incorrect and returning everything as 0, thus the totalItemPercentage will be Infinitiy due to the division by a subtotal of 0

* chore: Add missing changeset for @medusajs/medusa

* feat(medusa): Improve performance of Products domain (#3417)

* feat(medusa): Improve product update performances

* fix tests and update

* update mock repo

* improve repo

* cleanup

* fix

* cleanup + bulk emit + unit test fix

* improvements

* improve

* fix unit tests

* fix export

* fix product update handler

* enhance mock repo

* fix import integration

* fix end point tests

* revert mock repo product variant

* fix unit

* cleanup

* cleanup

* address feedback

* fix quotes in tests

* address feedback

* Create new-tips-mate.md

* use types

* chore: Remove integration-tests from changeset

* chore(release): v1.7.14

* chore(docs): Generated Docs Announcement Bar (automated) (#3489)

Co-authored-by: olivermrbl <[email protected]>

* fix(medusa): EventBusService.emit using Redis mock (#3491)

* Fix eventBusService.emit using redis mock

* revert gitignore

* enqueuer

* unit test add redis_url

* fix test

* chore(docs): Generated Services Reference (automated) (#3490)

Co-authored-by: olivermrbl <[email protected]>

* docs: publish restructure (#3496)

* docs: added features and guides overview page

* added image

* added version 2

* added version 3

* added version 4

* docs: implemented new color scheme

* docs: redesigned sidebar (#3193)

* docs: redesigned navbar for restructure (#3199)

* docs: redesigned footer (#3209)

* docs: redesigned cards (#3230)

* docs: redesigned admonitions (#3231)

* docs: redesign announcement bar (#3236)

* docs: redesigned large cards (#3239)

* docs: redesigned code blocks (#3253)

* docs: redesigned search modal and page (#3264)

* docs: redesigned doc footer (#3268)

* docs: added new sidebars + refactored css and assets (#3279)

* docs: redesigned api reference sidebar

* docs: refactored css

* docs: added code tabs transition

* docs: added new sidebars

* removed unused assets

* remove unusued assets

* Fix deploy errors

* fix incorrect link

* docs: fixed code responsivity + missing icons (#3283)

* docs: changed icons (#3296)

* docs: design fixes to the sidebar (#3297)

* redesign fixes

* docs: small design fixes

* docs: several design fixes after restructure (#3299)

* docs: bordered icon fixes

* docs: desgin fixes

* fixes to code blocks and sidebar scroll

* design adjustments

* docs: restructured homepage (#3305)

* docs: restructured homepage

* design fixes

* fixed core concepts icon

* docs: added core concepts page (#3318)

* docs: restructured homepage

* design fixes

* docs: added core concepts page

* changed text of different components

* docs: added architecture link

* added missing prop for user guide

* docs: added regions overview page (#3327)

* docs: added regions overview

* moved region pages to new structure

* docs: fixed description of regions architecture page

* small changes

* small fix

* docs: added customers overview page (#3331)

* docs: added regions overview

* moved region pages to new structure

* docs: fixed description of regions architecture page

* small changes

* small fix

* docs: added customers overview page

* fix link

* resolve link issues

* docs: updated regions architecture image

* docs: second-iteration fixes (#3347)

* docs: redesigned document

* design fixes

* docs: added products overview page (#3354)

* docs: added carts overview page (#3363)

* docs: added orders overview (#3364)

* docs: added orders overview

* added links in overview

* docs: added vercel redirects

* docs: added soon badge for cards (#3389)

* docs: resolved feedback changes + organized troubleshooting pages (#3409)

* docs: resolved feedback changes

* added extra line

* docs: changed icons for restructure (#3421)

* docs: added taxes overview page (#3422)

* docs: added taxes overview page

* docs: fix sidebar label

* added link to taxes overview page

* fixed link

* docs: fixed sidebar scroll (#3429)

* docs: added discounts overview (#3432)

* docs: added discounts overview

* fixed links

* docs: added gift cards overview (#3433)

* docs: added price lists overview page (#3440)

* docs: added price lists overview page

* fixed links

* docs: added sales channels overview page (#3441)

* docs: added sales overview page

* fixed links

* docs: added users overview (#3443)

* docs: fixed sidebar border height (#3444)

* docs: fixed sidebar border height

* fixed svg markup

* docs: added possible solutions to feedback component (#3449)

* docs: added several overview pages + restructured files (#3463)

* docs: added several overview pages

* fixed links

* docs: added feature flags + PAK overview pages (#3464)

* docs: added feature flags + PAK overview pages

* fixed links

* fix link

* fix link

* fixed links colors

* docs: added strategies overview page (#3468)

* docs: automated upgrade guide (#3470)

* docs: automated upgrade guide

* fixed vercel redirect

* docs: restructured files in docs codebase (#3475)

* docs: restructured files

* docs: fixed eslint exception

* docs: finished restructure loose-ends (#3493)

* fixed uses of backend

* docs: finished loose ends

* eslint fixes

* fixed links

* merged master

* added update instructions for v1.7.12

* docs: fixed discount details (#3499)

* docs: fix trailing slash causing 404 (#3508)

* docs: fix error during navigation (#3509)

* docs: removed the gatsby storefront guide (#3527)

* docs: removed the gatsby storefront guide

* docs: fixed query value

* chore(docs): Removed Docs Announcement Bar (automated) (#3536)

Co-authored-by: shahednasser <[email protected]>

* fix(medusa): Variant update should include the id for the listeners to be able to identify the entity (#3539)

* fix(medusa): Variant update should include the id for the listeners to be able to identify the entity

* fix unit tests

* Create brave-seahorses-film.md

* docs: fix admin redirects (#3548)

* chore(release): v1.7.15

* chore(docs): Generated Docs Announcement Bar (automated) (#3550)

Co-authored-by: olivermrbl <[email protected]>

* chore(docs): Generated Services Reference (automated) (#3551)

Automated changes by [create-pull-request](https:/peter-evans/create-pull-request) GitHub action

Co-authored-by: Oliver Windall Juhl <[email protected]>

* chore: updated READMEs of plugins (#3546)

* chore: updated READMEs of plugins

* added notice to plugins

* docs: added a deploy guide for next.js storefront (#3558)

* docs: added a deploy next.js guide

* docs: fix image zoom

* docs: fixes to next.js deployment guide to vercel (#3562)

* chore(workflows): Enable manual workflow in pre-release mode (#3566)

* chore(docs): Removed Docs Announcement Bar (automated) (#3598)

Co-authored-by: shahednasser <[email protected]>

* fix(medusa): Rounding issues on line item adjustments (#3446)

* chores(medusa): Attempt to fix discount rounding issues

* add migration

* update entities

* apply multipler factor properly

* fix discount service

* WIP

* fix rounding issues in discounts

* fix some tests

* Exclude raw_discount_total from responses

* fix adjustments

* cleanup response

* fix

* fix draft order integration

* fix order integration

* fix order integration

* address feedback

* fix test

* Create .changeset/polite-llamas-sit.md

* remove comment

---------

Co-authored-by: Oliver Windall Juhl <[email protected]>

* chore(workflows): Add release notification (#3629)

---------

Co-authored-by: pepijn-vanvlaanderen <[email protected]>
Co-authored-by: olivermrbl <[email protected]>
Co-authored-by: Adrien de Peretti <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: olivermrbl <[email protected]>
Co-authored-by: Carlos R. L. Rodrigues <[email protected]>
Co-authored-by: shahednasser <[email protected]>
Co-authored-by: Oliver Windall Juhl <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants