solidusio / solidus

🛒 Solidus, the open-source eCommerce framework for industry trailblazers.
https://solidus.io
Other
4.97k stars 1.29k forks source link

Order processing time increases dramatically with many inventory units #2342

Closed seand7565 closed 2 years ago

seand7565 commented 6 years ago

Solidus doesn't play well with orders that have many inventory units. In most cases, like t-shirt or book stores, this is not a problem. However for businesses that sell small hardware (screws or nuts), electrical parts(resistors or capacitors), or other low-cost items, this proves to be somewhat of a nightmare, as carts tend to blow up upon reaching a certain quantity.

Here is a gist with a rake job running an order to completion with many different inventory unit numbers. This results in the following:

"Benchmarking 10 line_items"
  1.700000   0.190000   1.890000 (  2.520435)
"Benchmarking 50 line_items"
  1.090000   0.060000   1.150000 (  1.182571)
"Benchmarking 100 line_items"
  0.870000   0.050000   0.920000 (  0.950034)
"Benchmarking 500 line_items"
  2.830000   0.090000   2.920000 (  2.947800)
"Benchmarking 1000 line_items"
  5.160000   0.070000   5.230000 (  5.265190)
"Benchmarking 2000 line_items"
 16.820000   0.110000  16.930000 ( 17.063924)
"Benchmarking 3000 line_items"
 31.510000   0.150000  31.660000 ( 31.708390)
"Benchmarking 4000 line_items"
 52.550000   0.160000  52.710000 ( 52.813332)
"Benchmarking 5000 line_items"
 83.710000   0.210000  83.920000 ( 84.014666)

In practice, our stores carts tend to blow up at anything past 3000 inventory units. We do have many wholesale customers so this poses a major problem for us. On a smaller note, the current inventory unit system also produces a lot of extra data - over 200K inventory units have been generated by our store in the last year.

In keeping with the scalable nature of Solidus, I believe that the order processing time should be the same regardless of the number of inventory units - a 500K inventory unit order should take just as long to process as a 5 inventory unit order. I know @jhawthorn did some work on this back in Spree and his solution was adding a quantity to inventory units instead of generating many inventory units for the same item based on the quantity. It was brilliant work, but never got merged into Spree. Is this something that we can explore doing to Solidus? Are there any other solutions that would work just as well? I'd love to hear some opinions and thoughts about this.

AdnanTheExcellent commented 6 years ago

I have been facing this issue for a while too. We have disabled inventory units because it completely blows up our site when enabled. We deal with manufacturing units so an order can have hundreds of thousands of inventory units. It looks like spree already did the quantity column implementation with this commit:

https://github.com/spree/spree/commit/0beb977b31e02adb34646afbb216ae6525f68f05

Is there a technical reason why this implementation has not been ported over to solidus (that i am unaware of) besides that nobody has gotten around to it? If not, I can try to tackle it but i'm fairly busy with my 9-5 and not sure if i'll have time to get it done.

wildbillcat commented 4 years ago

So in defense of inventory units, they are important if you are using solidus/spree as your warehouse management system for a vendor tracing perspective. The benefit of of them is if you are selling a product that you can source from 4 different vendors, if the manufacturer has to recall a batch, you can easily trace which customers are effected because the originating vendor will be associated to an inventory unit. If is also the basis for offering FIFO and FILO cogs tracking depending on what accounting method your practice.

That said offering an alternative inventory tracking system for those that don't have that requirement could work, but if anyone in the community uses the above features I'd be cautious to remove them. Out of curiosity is the problem with the inventory units that your order confirmation page isn't coming back to you quick enough because of the after_complete callback?

wildbillcat commented 4 years ago

Cross posting from the slack, if the count method is the culprit that is slowing things down, then counter_cache may be a solution here: https://blog.appsignal.com/2018/06/19/activerecords-counter-cache.html

seand7565 commented 4 years ago

Also x-posting from Slack, just for the record!

The problem isn't count itself - it's just that deprecating it is the first step in fixing the actual issue, which is that creating an individual inventory_unit for each quantity ordered leads to a broken checkout flow, given a large enough order. 2000+ item checkouts are not uncommon for places like hardware stores, that might sell screws or resistors that are pennies a piece. It's unscalable, and I'd like to fix it - because we all want Solidus to be as scalable as possible. Creating 2000+ databased objects while transitioning to the shipment page of checkout is kind of unreasonable.

I do see the value of inventory_units being flexible enough to represent individual stock - at the very least, to represent in_stock and backordered which is an important distinction. The fix that Spree put into place (that I'd like to replicate) is that inventory_units have a quantity , BUT also allow inventory_units to be very easily splittable - so if you wanted to keep track of what came from where, you 'd just split the inventory_units accordingly (10,000 inventory_units came from vendor X, 100 came from vendor Y, and 12 are backordered - that's three inventory_units total, representing 10,112 total inventory movements). You're still doing a count (or rather, sum(&:quantity) ) but you're only having to create 3 objects to represent 10K+ movements - it means the difference between a 2 second page load time and a 60+ second page load time.

aldesantis commented 4 years ago

I think @seand7565's point is valid, and the way we handle inventory units is one of the most common complaints for people using Solidus in a store that sells large volumes. AFAIK, @cedum just started working on an improvement around this!

wildbillcat commented 4 years ago

That makes sense. Thanks for the clarification.