[
https://issues.apache.org/jira/browse/OFBIZ-2353?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14117958#comment-14117958
]
Jacques Le Roux commented on OFBIZ-2353:
----------------------------------------
Ha indeed, I did not review your patch then. First some minor remarks about it.
If we change stuff like
{code}
- if (bankSize > SequenceBank.maxBankSize) bankSize =
SequenceBank.maxBankSize;
+ if (bankSize > SequenceBank.maxBankSize)
+ bankSize = SequenceBank.maxBankSize;
{code}
then we should rather do
{code}
- if (bankSize > SequenceBank.maxBankSize) bankSize =
SequenceBank.maxBankSize;
+ if (bankSize > SequenceBank.maxBankSize) {
+ bankSize = SequenceBank.maxBankSize;
+ }
{code}
or let it as is. Because do we really need to change that? I mean at large it's
more lines to review, and I'm ok with the old form. Actually I prefer the
convention we agreed about but I also prefer the old form above yours, more
legible and especially less error prone.
About your functional changes, sorry to say but they miss the good comments
Jacopo put ;).
I don't like much the while(true) continue pattern, I find it less readable at
1st glance than Jacopo's straightforward, sequential way. Maybe only because it
misses comments?
I appreciate the prepared statement, even if it's only used once.
Now I think I understand the rational behind your pattern. It works following
the way the lock works (actually counter way, would be easier to read with few
comments). But apart that (less lines), what does it add above Jacopo"s way?
> 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)
