Thanks Swapnil for adding the use case. After this it looks like, this is kind of scenario when we couldn't lean on the ATP. Which should be discussed and addressed. But now I'm sure that what Suraj suggested makes sense we can go with the improvement Suraj suggested.
In isolation we can discuss and try to address the race condition issue and follow the steps. - Add script to replicate the issue multiple multiple times. - Discuss and finalize the fix. - Provide fix. I would like to help in the race condition issue Swapnil shared. +1 for Suraj to move ahead for the improvement. Rishi Solanki Sr Manager, Enterprise Software Development HotWax Systems Pvt. Ltd. Direct: +91-9893287847 http://www.hotwaxsystems.com www.hotwax.co On Wed, Apr 11, 2018 at 11:08 AM, Swapnil Shah < swapnil.s...@hotwaxsystems.com> wrote: > There are certain business cases around order promising where we found that > systemic ATP hasn't proved that much reliable. Especially when its business > decision to not accept or promise more orders than allocated units of > supply > for sale. > > For example, during heavy load(ordering) there could be instances when > higher number of open orders/carts are competing for same systemic ATP at > any given point of time. In such scenarios due to any reason if rate of > performing systemic reservations lags behind the rate of ordering than > systemic ATP would also keep lagging behind the actual allocation being > made > with respect to QOH. Thus system would always keep on accepting orders and > promising them unless systemic ATP goes down to zero (but in reality the > QOH > Is already exhausted way before than systemic ATP went to zero). It leads > to > the problem of "Over Promising" and eventually higher than acceptable > number > of backorders to honor for business. In the hindsight it looks like this > could be one of the reason why the additional check on QOH was in place > before. > > I am not sure if it’s the best way, but one of the possible alternative we > tried to handle such cases was by grounding the order creation logic based > on the fact whether there is positive "Available to Order (ATO)" at the > time > of order submission or adding items to cart rather than ATP. At high level > ATO for any given SKU could be determined on run time as follows: > ATO = QOH + Incoming Shipments(Scheduled Receipts) - (Total unshipped units > on Open Orders & Carts) > > I hope such cases could help in providing more holistic view while > leveraging or relying upon the reservation logic. > > Thanks, > Swapnil > > -----Original Message----- > From: Jacopo Cappellato <jacopo.cappell...@hotwaxsystems.com> > Sent: Tuesday, April 10, 2018 1:47 PM > To: dev@ofbiz.apache.org > Subject: Re: Check for only QOH while doing reservations > > Thanks Suraj, > > after reviewing that old commit I am inclined to think that the change you > are suggesting makes sense. > Before that old commit all the inventory items (regardless of their type > and > qty) were selected and there was logic to iterate thru the result set and > exclude the ones with the wrong type and reserve only the ones with ATP. > With that commit the type constraint was added to the query and also an > additional constraint on QOH (rather than ATP): maybe at that time there > was > code requiring it or maybe it was done that way to be extra careful. > I think we can now proceed as you suggest but before we do we should review > the code that calls the following services: > reserveProductInventoryByFacility > reserveProductInventoryByContainer > > and make sure that the change will not impact them negatively. > > Kind regards, > > Jacopo > > > On Mon, Apr 9, 2018 at 3:27 PM, Suraj Khurana < > suraj.khur...@hotwaxsystems.com> wrote: > > > Thanks Scott, > > > > I looked around and found some relevant commit. > > IMO, it has been mistakenly committed as commit log also doesn't shows > > any functional change in commit. > > Here > > <https://svn.apache.org/viewvc/ofbiz/trunk/applications/product/script > > / org/ofbiz/product/inventory/InventoryReserveServices.xml? > > r1=650764&r2=650763&pathrev=650764> > > is the link for reference. > > > > -- > > Thanks and Regards, > > *Suraj Khurana* | Omni-channel OMS Technical Expert HotWax Commerce > > by HotWax Systems Plot no. 80, Scheme no. 78, Vijay Nagar, Indore, > > M.P. India 452010 > > > > > > On Sat, Apr 7, 2018 at 3:24 AM, Scott Gray > > <scott.g...@hotwaxsystems.com> > > wrote: > > > > > Hi Suraj, > > > > > > I haven't reviewed the code in question so I don't have any comment > > > at > > this > > > stage. But one thing I want to point out is that OFBiz has many > > > years of history available in commit logs, jira and mailing lists. > > > It's often > > quite > > > a simple task to look back over that history and determine why a > > > certain thing was done a certain way. > > > > > > As part of proposing a change to existing functionality it is > > > extremely useful to anyone who might review the proposal to have > > > some of that > > context > > > provided with the proposal. > > > > > > In this case it could be a simple matter of a past mistake being > > overlooked > > > until now, or it could be that using QOH was found to be beneficial > > > for some reason that isn't immediately obvious. But without first > > researching, > > > we can't ever be sure of the answer. > > > > > > Regards > > > Scott > > > > > > On Fri, 6 Apr 2018, 18:25 Suraj Khurana, <suraj.khurana@hotwaxsystems. > > com> > > > wrote: > > > > > > > Hello, > > > > > > > > While checking around code around inventory reservations, I was > > surprised > > > > to see that *reserveProductInventory *service only checks for QOH > > > quantity > > > > greater than one apart from that when > > > > *reserveFromInventoryItemInline > > > *is > > > > called, it checks for ATP confirming system to behave as required. > > > > > > > > Everything works fine but this is redundant code and we can have > > > > check > > > for > > > > ATP at top level so make reservations logic works faster. Is there > > > > any other specific case I am missing or we can improve this flow > > > > by adding > > > ATP > > > > check at *reserveProductInventory* service as well. > > > > > > > > We can check QOH being on safer side, but ideally a system will > > > > always > > > have > > > > lesser ATP than QOH and logically we should only check for ATP > > > > while > > > doing > > > > reservations. > > > > > > > > -- > > > > Thanks and Regards, > > > > *Suraj Khurana* | Omni-channel OMS Technical Expert HotWax > > > > Commerce by HotWax Systems Plot no. 80, Scheme no. 78, Vijay > > > > Nagar, Indore, M.P. India 452010 > > > > > > > > > >
