Check for only QOH while doing reservations

[Suraj Khurana]

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

[Scott Gray-3]

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, <[hidden email]>
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
>

[Suraj Khurana]

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 <[hidden email]>
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, <[hidden email]>
> 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
> >
>

[Jacopo Cappellato-5]

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 <
[hidden email]> 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 <[hidden email]>
> 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
> > >
> >
>

[swapnil]

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 <[hidden email]>
Sent: Tuesday, April 10, 2018 1:47 PM
To: [hidden email]
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 <
[hidden email]> 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
> <[hidden email]>
> 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
> > >
> >
>

[Rishi Solanki]

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: [phone]
http://www.hotwaxsystems.com
www.hotwax.co

On Wed, Apr 11, 2018 at 11:08 AM, Swapnil Shah <
[hidden email]> 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 <[hidden email]>
> Sent: Tuesday, April 10, 2018 1:47 PM
> To: [hidden email]
> 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 <
> [hidden email]> 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
> > <[hidden email]>
> > 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
> > > >
> > >
> >
>

[Suraj Khurana]

Thanks everyone for your input.

Here <https://issues.apache.org/jira/browse/OFBIZ-10337> is the ticket
created for the same.

--
Thanks and Regards,
*Suraj Khurana* | Omni-channel OMS Technical Expert
HotWax Commerce <http://www.hotwax.co/>  by  HotWax Systems
<http://www.hotwaxsystems.com/>
Plot no. 80, Scheme no. 78, Vijay Nagar, Indore, M.P. India 452010
Cell phone: [phone]

On Wed, Apr 11, 2018 at 12:56 PM, Rishi Solanki <[hidden email]>
wrote:

> 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: [phone]
> http://www.hotwaxsystems.com
> www.hotwax.co
>
> On Wed, Apr 11, 2018 at 11:08 AM, Swapnil Shah <
> [hidden email]> 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 <[hidden email]>
> > Sent: Tuesday, April 10, 2018 1:47 PM
> > To: [hidden email]
> > 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 <
> > [hidden email]> 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
> > > <[hidden email]>
> > > 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
> > > > >
> > > >
> > >
> >
>

[Vaibhav Jain]

+1 Suraj

I agree with Rishi, We should start another mail thread to discuss race
condition.

IMO, At the time of reservation, we should check for ATP instead of QOH(As
Suraj suggest).

Vaibhav Jain
Sr. Enterprise Software Engineer
HotWax Systems
m: 782-834-1900 e: [hidden email]
<[hidden email]>

On Thu, Apr 12, 2018 at 4:31 PM, Suraj Khurana <
[hidden email]> wrote:

> Thanks everyone for your input.
>
> Here <https://issues.apache.org/jira/browse/OFBIZ-10337> is the ticket
> created for the same.
>
> --
> Thanks and Regards,
> *Suraj Khurana* | Omni-channel OMS Technical Expert
> HotWax Commerce <http://www.hotwax.co/>  by  HotWax Systems
> <http://www.hotwaxsystems.com/>
> Plot no. 80, Scheme no. 78, Vijay Nagar, Indore, M.P. India 452010
> Cell phone: [phone]
>
> On Wed, Apr 11, 2018 at 12:56 PM, Rishi Solanki <[hidden email]>
> wrote:
>
> > 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: [phone]
> > http://www.hotwaxsystems.com
> > www.hotwax.co
> >
> > On Wed, Apr 11, 2018 at 11:08 AM, Swapnil Shah <
> > [hidden email]> 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 <[hidden email]>
> > > Sent: Tuesday, April 10, 2018 1:47 PM
> > > To: [hidden email]
> > > 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 <
> > > [hidden email]> 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
> > > > <[hidden email]>
> > > > 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
> > > > > >
> > > > >
> > > >
> > >
> >
>

[Jacques Le Roux]

+1 Vaibhav

Jacques


Le 14/04/2018 à 08:35, Vaibhav Jain a écrit :

> +1 Suraj
>
> I agree with Rishi, We should start another mail thread to discuss race
> condition.
>
> IMO, At the time of reservation, we should check for ATP instead of QOH(As
> Suraj suggest).
>
> Vaibhav Jain
> Sr. Enterprise Software Engineer
> HotWax Systems
> m: 782-834-1900 e: [hidden email]
> <[hidden email]>
>
> On Thu, Apr 12, 2018 at 4:31 PM, Suraj Khurana <
> [hidden email]> wrote:
>
>> Thanks everyone for your input.
>>
>> Here <https://issues.apache.org/jira/browse/OFBIZ-10337> is the ticket
>> created for the same.
>>
>> --
>> Thanks and Regards,
>> *Suraj Khurana* | Omni-channel OMS Technical Expert
>> HotWax Commerce <http://www.hotwax.co/>  by  HotWax Systems
>> <http://www.hotwaxsystems.com/>
>> Plot no. 80, Scheme no. 78, Vijay Nagar, Indore, M.P. India 452010
>> Cell phone: [phone]
>>
>> On Wed, Apr 11, 2018 at 12:56 PM, Rishi Solanki <[hidden email]>
>> wrote:
>>
>>> 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: [phone]
>>> http://www.hotwaxsystems.com
>>> www.hotwax.co
>>>
>>> On Wed, Apr 11, 2018 at 11:08 AM, Swapnil Shah <
>>> [hidden email]> 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 <[hidden email]>
>>>> Sent: Tuesday, April 10, 2018 1:47 PM
>>>> To: [hidden email]
>>>> 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 <
>>>> [hidden email]> 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
>>>>> <[hidden email]>
>>>>> 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
>>>>>>>