[
https://issues.apache.org/jira/browse/OFBIZ-2353?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14118017#comment-14118017
]
Adrian Crum commented on OFBIZ-2353:
------------------------------------
Jacopo,
I apologize if I offended you - that was not my intention. I was trying to
propose an alternate method of generating sequences and I was hoping we could
discuss the pros and cons of each method. I respect and appreciate the time you
have invested in this rewrite, and I am not trying make you look dumb. From
your perspective, you simply want to undo Jacques' changes and fix some
thread-safety issues (or they might have been data-race conditions). From my
perspective, I see this as an opportunity to re-evaluate the algorithm and the
class as a whole to see if we can improve it. That is the motivation behind my
comments - they are not meant to be some kind of personal attack.
I understand you are done with this issue and you want to move on, so there is
no need for you to respond further. But I would like to clear up some
misinformation for anyone else who is interested.
The current sequencer implementation has been documented as "an Ethernet-like
collision detection design" - but it is not. In the CSMA/CD version of
Ethernet, multiple devices will attempt to send their data on the medium, and
if there is a collision (more than one device sending data simultaneously),
then each device waits a random period of time and tries again.
The current sequencer design locks a record, reads it, increments the sequence
value, and then writes the new value. That is nothing like CSMA/CD. A
"CSMA/CD-like" design would attempt to update the record without locking it
first, and if the update fails, it will retry. That is the design I proposed.
The current sequencer design is a blocking one: while one process holds the
lock, other processes are blocked - waiting for the lock to be released. The
design I proposed is non-blocking: there are no locks - so processes are not
blocked.
The disadvantage to the non-blocking design is when an update fails - the
record needs to be read and the update is tried again. This can result in more
round trips to the database.
The disadvantage to the blocking design is the window of opportunity for
failure after a lock is acquired and before it is released. In a best case
scenario, the transaction manager rolls back the transaction and the blocked
processes can proceed. In a worse case scenario, something goes wrong during
rollback and the record is locked indefinitely. From my perspective this is a
genuine concern and it is not something I made up just to be argumentative.
Jacopo asked me to provide a unit test to prove this can happen. I would like
to, but I don't know how.
There was some discussion about performance and which design performs better.
From a pure design perspective, there is no way to measure the difference. Each
design will perform better than the other depending upon the update scenario.
Regarding the performance of the patch I provided, I agree it performed worse
than the current implementation - but that is because I was proposing a design,
the patch was not a "finished product." Since my design involves a retry loop,
I looked inside the loop to see what could be slowing it down. I came to the
conclusion it was String concatenation combined with JDBC driver parsing of the
SQL. Once I moved those things outside the retry loop, my design performed as
well as the current implementation.
If anyone wants to explore these topics further, then please continue this
discussion on the developers mailing list. If anyone wants to continue working
on the design I proposed, then please do so. There is a bug in the initial
sequence value, and as Jacopo mentioned, the PreparedStatement needs to be
closed.
> SequenceUtil may generate duplicate IDs in Load Balancing mode
> ---------------------------------------------------------------
>
> Key: OFBIZ-2353
> URL: https://issues.apache.org/jira/browse/OFBIZ-2353
> Project: OFBiz
> Issue Type: Bug
> Components: framework
> Affects Versions: Release Branch 4.0, Release Branch 09.04, Trunk
> Reporter: Philippe Mouawad
> Assignee: Jacopo Cappellato
> Priority: Critical
> Fix For: Release Branch 10.04, Upcoming Branch
>
> Attachments: OFBIZ-2353 SELECT FOR UPDATE solution.patch, OFBIZ-2353
> SELECT FOR UPDATE solution.patch, OFBIZ-2353-AC.patch, OFBIZ-2353-AC.patch
>
>
> If Ofbiz is deploy on 2 servers in Load Balancing Mode
> SequenceUtil will generate duplicate IDs because synchronization is done at
> JVM level instead of doing it in DB.
> A good replacement implementation would be:
> org.hibernate.id.enhanced.TableGenerator
> But it would involve a dependency on Hibernate
> Philippe
> www.ubik-ingenierie.com
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
