--- On Fri, 5/21/10, Matt Warnock <[email protected]> wrote: > Adrian: > > I had been following it (with some interest) from the > start, but went > back and read it again. Also reviewed the JIRA issue, > which I had not > previously. I have also reviewed the patch you > submitted, though I am > not altogether certain what "special" invoice order > sequencing means in > this context, and in any event, I am no XML or ofbiz expert > (yet). But > is seems that IF "special" sequencing is in effect, then > this race > condition occurs.
The special sequencing can be found in the simple methods that are changed in the patch. > Now, MOST businesses will require that every invoice number > is unique, > and MANY will further require that all order/invoice/check > numbers be > accounted for, even on prenumbered forms (no "gaps") for > auditing > purposes. Is that "special" to ofbiz? > > Alternatively, some businesses I have seen enforce a > particular format > on the invoice/order/document number (like OOOYYYYMMDDSSS, > where OOO is > the office number, followed by a date, and SSS is a > sequence number > within the day), and maybe that is where the trouble > lies. In the US, > Social Security numbers are generated on a "special" > pattern like this. > > In either event, I remain convinced, and on reviewing the > thread, David > Jones seems to agree, that this problem cannot be easily > solved except > with atomic SQL transactions. Is there some reason > they are not being > used in this situation? Or are they in use, and is > the OP using a > REALLY broken JDBC? Or are they in use, but a record write > has > inadvertently snuck outside the transaction? No, atomic transactions are not being used in this case. The invoice/order ID generation code is in a simple method, and simple methods default to having their own transaction. So, the ID can be generated successfully, but the invoice/order creation could fail. > At our company, we have two people putting orders in all > morning (and we > are a very small company), and the chance of a collision in > your > scenario is NOT rare, but quite high indeed, as the OP has > already > pointed out, even on a single server with multiple > users. But we would > only find this out after rollout, not during single-user > testing, and it > would be an apparently random system lockup, which would be > frustrating > to say the least. Kudos to the OP for spotting the > real nature of the > problem-- race conditions are often hard to identify. > :-) > > The issue is not that the sequence generator is "slow" to > store the new > invoice number, but rather that "last ID" read, the "new > ID" generator, > and the "new ID" write are not ALL encased inside a > transaction that > will ensure that the database is ALWAYS internally > consistent. The > "race" occurs because there is no enclosing transaction, > and the "race" > will never be completely controlled by being "fast" > enough. > > If there is a long lag between read and write (as where a > user is > entering order line items) then you either have to 1) do > the > read/increment/write at the END of the data entry process, > or 2) face > the high likelihood that two entry processes will collide, > and one of > them will have to roll back and then be renumbered. > > Most software I have seen is not very bright about this, > and it reads > and generates the new order number first, displaying it on > the input > screen as data is entered. That is not the > SQL-friendly way, and asks > for trouble. Users like it, because they often need > to write that > number down on the data entry form-- they just need > to be trained to > get that order ID as the last thing they do. :( I'm > strongly hoping > ofbiz will conform to better practices here, as it has > elsewhere. > > Whether you use a SQL standard "sequence" or not (or even > the mysql > "autoincrement" abomination), and regardless of the primary > key status > of the orderID (any SQL "unique" declaration could have the > same effect, > and a well designed order table should at LEAST have a > unique orderID, > whether it is part of the primary key or not), SQL > transactions should > resolve this issue easily and scalably. > > On the other hand, using locking or semaphores in the > application code > is reinventing the SQL wheel and fraught with possible > error. As David > suggested, there may be multiple front ends talking to a > single > database, so a SQL solution is best. I am merely > respectfully > suggesting that it would be better to let SQL do what it > does best, and > atomic transactions are the essence of good SQL. > > Hope this helps explain where I am coming from. > > -- > Matt Warnock <[email protected]> > RidgeCrest Herbals, Inc. > > On Thu, 2010-05-20 at 22:26 -0700, Adrian Crum wrote: > > Matt, > > > > If you follow the thread from the start and visit the > Jira issue, you will see that we are discussing a very > specific problem - not transactions or sequences in > general. > > > > To summarize: There is code in OFBiz that generates > sequential ID numbers for orders and invoices. That code > does not follow the established pattern for generating > sequences, and as a result it will produce non-sequential > IDs. A Jira issue has been created and we are working on the > problem. > > > > -Adrian > > > > --- On Thu, 5/20/10, Matt Warnock <[email protected]> > wrote: > > > > > From: Matt Warnock <[email protected]> > > > Subject: Re: QuoteId and OrderId sequence, > possible race condition? > > > To: [email protected] > > > Date: Thursday, May 20, 2010, 5:15 PM > > > Isn't this exactly the kind of thing > > > that SQL transactions, sequences, > > > and foreign key constraints are designed to > solve? > > > > > > If the thread is using transactions properly, the > whole > > > transaction > > > should either be rolled back, or completed. > Either > > > way, the database > > > SHOULD be in a wholly consistent state. But > it > > > obviously isn't. > > > > > > So either we are doing something outside a > transaction, > > > that really > > > should be done INSIDE a transaction, or the > database has a > > > seriously > > > flawed implementation of transactions. > > > -- > > > Matt Warnock <[email protected]> > > > RidgeCrest Herbals, Inc. > > > > > > On Thu, 2010-05-20 at 12:43 -0700, Adrian Crum > wrote: > > > > I doubt there will be any need for > additional > > > synchronization. > > > > > > > > I think I know what's going on in your > system. My > > > previous scenario > > > > wasn't correct because I thought the ID was > a part of > > > the primary key > > > > and there was an exception thrown while > storing the > > > new value - due to a > > > > duplicate key violation. Those IDs are not a > part of > > > the primary key, so > > > > that isn't the cause. > > > > > > > > Instead, I believe the ID is updated > successfully, but > > > a thread is slow > > > > in storing the new ID value - which ends up > > > decrementing the ID in the > > > > database instead of incrementing it. Then > the invoice > > > generation fails > > > > due to a duplicate key violation. From that > point on, > > > every attempt to > > > > increment the ID value and create a new > invoice with > > > it will fail > > > > because the new ID value already exists in > the invoice > > > file. > > > > > > > > -Adrian > > > > > > > > On 5/20/2010 11:57 AM, James McGill wrote: > > > > > On Thu, May 20, 2010 at 10:46 AM, > Adrian > > > Crum<[email protected]> > > > > wrote: > > > > > > > > > >> James, > > > > >> > > > > >> I uploaded a patch that might fix > your > > > problem: > > > > >> > > > > >> > > > > > Thanks Adrian. Your approach adds > JVM-based > > > synchronization to minilang > > > > > simple methods. > > > > > > > > > > I still wonder if this means the create > services > > > need to be synchronized and > > > > > not just the id generators. > > > > > > > > > > I'll try it out and keep you posted. > > > > > > > > > > > > > > > James > > > > > > > > > > > > > > > > > > >
