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

[Bug]: Performance issues around price lists #11440

Open
Edsparr opened this issue Feb 13, 2025 · 6 comments
Open

[Bug]: Performance issues around price lists #11440

Edsparr opened this issue Feb 13, 2025 · 6 comments

Comments

@Edsparr
Copy link

Edsparr commented Feb 13, 2025

Package.json file

-

Node.js version

Database and its version

Operating system name and version

Browser name

What happended?

When having large price lists batchPriceListsWorkflow crumbles.

When using batchPriceListsWorkflow it uses different workflow steps to update, create and delete. (

)

These steps (create prices for example) call this workflow. https://github.com/medusajs/medusa/blob/develop/packages/core/core-flows/src/price-list/workflows/create-price-list-prices.ts#L59

which in turn calls validatePriceListsStep() (https://github.com/medusajs/medusa/blame/develop/packages/core/core-flows/src/price-list/steps/validate-price-lists.ts#L23)

This workflow fetches all the prices related to the price list. In lager price lists I have encountered this workflow taking several minutes to complete and will eventually stop working. I have not yet looked at possible solutions but I imagine we could probably bound this wildcard select to something.

Expected behavior

Actual behavior

Link to reproduction repo

@riqwan
Copy link
Contributor

riqwan commented Feb 13, 2025

@Edsparr, can you share some stats (count of price lists, prices, price rules, etc), what the input is for the query that breaks and maybe a db dump (price module tables alone if privacy is a concern) to riqwan[@]medusajs[dot]com?

@Edsparr
Copy link
Author

Edsparr commented Feb 13, 2025

Hi riqwan, thanks for a quick response! I don't have tracing setup currently (I can set it up if you feel it is still necessary). I also can not send my database since it uses real life data. Though I am pretty sure it has to do with this giant select. When I query my price table with prices restricted to the big price list it comes out to 36563 rows.

It seems pretty reasonable that this huge fetch causes some performance issues?

@riqwan
Copy link
Contributor

riqwan commented Feb 13, 2025

I understand @Edsparr . I'm not looking for tracing, but to replicate this issue, I would need the following:

select count(*) from price_list;
select count(*) from price;
select count(*) from price left join price_list on price_list.id = price.price_list_id

Additionally, when you're calling batchPriceListsWorkflow, what does your input look like? i.e. how many price lists are you updating in one call and how many prices underneath?

@Edsparr
Copy link
Author

Edsparr commented Feb 13, 2025

I understand @Edsparr . I'm not looking for tracing, but to replicate this issue, I would need the following:

select count(*) from price_list;
select count(*) from price;
select count(*) from price left join price_list on price_list.id = price.price_list_id

Additionally, when you're calling batchPriceListsWorkflow, what does your input look like? i.e. how many price lists are you updating in one call and how many prices underneath?

I understand. I apologize for not starting of with some clear reproduction instructions

How to replicate:

  • Create a price list.
  • Populate the price table with ~35k entries restricted to the price list.
  • Call batchPriceListPricesWorkflow() with the id = a big price list. create, update, delete input can be empty.

Even though the input is empty the validate function will be called which is why this workflow takes several minutes to complete even though no input was provided.

@riqwan
Copy link
Contributor

riqwan commented Feb 13, 2025

Got it, thanks!

Just to confirm, this should suffice with a price list with 35k prices, right?

await batchPriceListPricesWorkflow.run({
    input: {
      data: {
        id: "big-price-list",
        create: [],
        update: [],
        delete: [],
      },
    },
  })

@Edsparr
Copy link
Author

Edsparr commented Feb 13, 2025

Got it, thanks!

Just to confirm, this should suffice with a price list with 35k prices, right?

await batchPriceListPricesWorkflow.run({
    input: {
      data: {
        id: "big-price-list",
        create: [],
        update: [],
        delete: [],
      },
    },
  })

Yes, I believe so!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants