[jira] [Commented] (OFBIZ-3557) Enforced sequence does not work with concurrent access

[Nicolas Malin (Jira)]

    [ https://issues.apache.org/jira/browse/OFBIZ-3557?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13040108#comment-13040108 ]

Jacopo Cappellato commented on OFBIZ-3557:
------------------------------------------

I have spent some time studying the problem and doing some research.

First of all, I agree that the current solution has major flaws and it should be completely redesigned: my preference would be to keep the default id creation for the invoiceId pk as is and add a new non-pk field to Invoice: "invoiceName" or "invoiceNumber" or "invoiceProtocolNumber" etc... The idea is that, based on PartyAcctgPreference, the system will set the invoiceName only right after the invoice has been successfully created (based on the invoiceName of the previous invoice).

That said, I would like to share my thoughts about the current issue.
In my opinion, even if it is definitely true that the issue is caused by a race condition on PartyAcctgPreference, I don't think this is caused by the fact that the getNextInvoiceId runs in a separate transaction; it shouldn't be the case because OFBiz uses the same transaction for all the services unless a new transaction is explicitly required (require-new-transaction="true").
I think that the problem is caused by the default isolation level used by OFBiz: "ReadCommitted" (this is set in entityengine.xml). Here is the description from Wikipedia:
======================
READ COMMITTED
In this isolation level, a lock-based concurrency control DBMS implementation keeps write locks (acquired on selected data) till the end of the transaction, but read locks are released as soon as the SELECT operations is performed (so non-repeatable reads phenomenon can occur in this isolation level -see below-). As in the previous level, range-locks are not managed.
======================
This means that if I run "createInvoice" for invoice A and B and PartyAcctgPreference.lastInvoiceNumber is initially 10:
createInvoice for A starts transaction A
createInvoice for B starts transaction B
createInvoice for A calls getNextInvoiceId using transaction A
createInvoice for B calls getNextInvoiceId using transaction B
getNextInvoiceId for A reads lastInvoiceNumber = 10, increment it (11) and store (not committed), the record becomes write locked
getNextInvoiceId for B reads lastInvoiceNumber = 10 (because of ReadCommitted isolation level), increment it (11) but the record is write locked by transaction A and so it waits to store the value (not committed)
createInvoice for A assigns InvoiceId 11 and store the invoice; transaction A is committed and PartyAcctgPreference and Invoice are persisted and unlocked; now we have invoice A with invoiceId 11 and PartyAcctgPreference.lastInvoiceNumber=11
getNextInvoiceId for B can continue now, and it stores the PartyAcctgPreference.lastInvoiceNumber=11 (first problem)
createInvoice for B assigns InvoiceId 11 and when attempts to store the invoice it gets a duplicated PK error;
transaction B is rolled back: invoice B is not stored and also PartyAcctgPreference is rolled back: I suspect that it could roll back to the original version read by B instead of the newly committed version A ***

The end result is that in the system we will have: Invoice for A with id 11; PartyAcctgPreference.lastInvoiceNumber=10
*** = this is the step I am not 100% sure about

How can we fix this?
You could try with the following hack: set require-new-transaction="true" in the service definition for getNextInvoiceId.
This should cause the following behaviour:

createInvoice for A starts transaction A
createInvoice for B starts transaction B
createInvoice for A calls getNextInvoiceId using new transaction A1
createInvoice for B calls getNextInvoiceId using new transaction B1
getNextInvoiceId for A reads lastInvoiceNumber = 10, increment it (11) and store; transaction A1 is committed getNextInvoiceId for B reads lastInvoiceNumber = 11, increment it (12) and store;
transaction B1 is committed createInvoice for A assigns InvoiceId 11 and store the invoice;
transaction A is committed and Invoice is persisted and unlocked;
now we have invoice A with invoiceId 11 and PartyAcctgPreference.lastInvoiceNumber=12
createInvoice for B assigns InvoiceId 12 and when attempts to store the invoice it gets a duplicated PK error;
transaction B is rolled back: invoice B is not stored but PartyAcctgPreference is NOT rolled back The end result is that in the system we will have: Invoice for A with id 11; PartyAcctgPreference.lastInvoiceNumber=12

With this solution we will get gaps (Invoice C will get invoiceId=13) but not errors (errors could still happen but it will be very rare because it is very unlikely that we will get the race condition because getNextInvoiceId is very quick, while createInvoice is a slow service).

> Enforced sequence does not work with concurrent access
> ------------------------------------------------------
>
>                 Key: OFBIZ-3557
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-3557
>             Project: OFBiz
>          Issue Type: Bug
>          Components: framework
>    Affects Versions: Release Branch 09.04, SVN trunk
>            Reporter: Wickersheimer Jeremy
>         Attachments: OFBIZ-3557-1.patch, OFBIZ-3557-2.patch
>
>
> There is a fundamental issue with enforced sequences (for orders, invoices, etc ..) and concurrency.
> For example if two users are creating an order at the same time one of them will see the creation fail with a PK error. The problem is that the "getNextXXXId" rely on the party accounting preference entity, but there is absolutely no guarantee that the last number in the sequence gets updated before another service can read it.
> This is at best very annoying when used only internally but may be unpractical for e-commerce sites.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira