You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by ja...@apache.org on 2014/09/03 08:34:54 UTC

svn commit: r1622170 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/SequenceUtil.java

Author: jacopoc
Date: Wed Sep  3 06:34:54 2014
New Revision: 1622170

URL: http://svn.apache.org/r1622170
Log:
Improved formatting, comments and variable names.
A failed insert due to duplicated pk will generate an SQLException rather than returning 0 from executeStatement(), so changed the code to reflect this.

Modified:
    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/SequenceUtil.java

Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/SequenceUtil.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/SequenceUtil.java?rev=1622170&r1=1622169&r2=1622170&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/SequenceUtil.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/SequenceUtil.java Wed Sep  3 06:34:54 2014
@@ -155,9 +155,19 @@ public class SequenceUtil {
             this.fillBank(staggerMax);
         }
 
+        /*
+           The algorithm to get the new sequence id in a thread safe way is the following:
+           1 - run an update with no changes to get a lock on the record
+               1bis - if no record is found, try to create and update it to get the lock
+           2 - select the record (now locked) to get the curSeqId
+           3 - increment the sequence
+           The three steps are executed in one dedicated database transaction.
+         */
         private void fillBank(long stagger) {
             // no need to get a new bank, SeqIds available
-            if ((curSeqId + stagger) <= maxSeqId) return;
+            if ((curSeqId + stagger) <= maxSeqId) {
+                return;
+            }
 
             long bankSize = this.bankSize;
             if (stagger > 1) {
@@ -165,7 +175,9 @@ public class SequenceUtil {
                 bankSize = stagger * defaultBankSize;
             }
 
-            if (bankSize > maxBankSize) bankSize = maxBankSize;
+            if (bankSize > maxBankSize) {
+                bankSize = maxBankSize;
+            }
 
             Transaction suspendedTransaction = null;
             try {
@@ -192,34 +204,35 @@ public class SequenceUtil {
                         throw new GenericEntityException("Unable to esablish a connection with the database, connection was null...");
                     }
 
-                    String sql = null;
                     try {
                         stmt = connection.createStatement();
-
-                        // run an update with no changes to get a lock on the record
+                        String sql = null;
+                        // 1 - run an update with no changes to get a lock on the record
                         if (stmt.executeUpdate(updateForLockStatement) <= 0) {
-                            Debug.logWarning("First select failed: will try to add new row, result set was empty for sequence [" + seqName + "] \nUsed SQL: " + sql + " \n ", module);
+                            Debug.logWarning("Lock failed; no sequence row was found, will try to add a new one for sequence: " + seqName, module);
                             sql = "INSERT INTO " + SequenceUtil.this.tableName + " (" + SequenceUtil.this.nameColName + ", " + SequenceUtil.this.idColName + ") VALUES ('" + this.seqName + "', " + startSeqId + ")";
-                            if (stmt.executeUpdate(sql) <= 0) {
+                            try {
+                                stmt.executeUpdate(sql);
+                            } catch (SQLException sqle) {
                                 // insert failed: this means that another thread inserted the record; then retry to run an update with no changes to get a lock on the record
                                 if (stmt.executeUpdate(updateForLockStatement) <= 0) {
                                     // This should never happen
-                                    throw new GenericEntityException("No rows changed when trying insert new sequence row with this SQL: " + sql);
+                                    throw new GenericEntityException("No rows changed when trying insert new sequence: " + seqName);
                                 }
+
                             }
                         }
-                        // select the record (now locked) to get the curSeqId
+                        // 2 - select the record (now locked) to get the curSeqId
                         rs = stmt.executeQuery(selectSequenceStatement);
-                        boolean gotVal = false;
-                        if (rs.next()) {
+                        boolean sequenceFound = rs.next();
+                        if (sequenceFound) {
                             curSeqId = rs.getLong(SequenceUtil.this.idColName);
-                            gotVal = true;
                         }
                         rs.close();
-                        if (!gotVal) {
-                            throw new GenericEntityException("Failed to find the sequence record: " + sql);
+                        if (!sequenceFound) {
+                            throw new GenericEntityException("Failed to find the sequence record for sequence: " + seqName);
                         }
-                        // increment the sequence
+                        // 3 - increment the sequence
                         sql = "UPDATE " + SequenceUtil.this.tableName + " SET " + SequenceUtil.this.idColName + "=" + SequenceUtil.this.idColName + "+" + bankSize + " WHERE " + SequenceUtil.this.nameColName + "='" + this.seqName + "'";
                         if (stmt.executeUpdate(sql) <= 0) {
                             throw new GenericEntityException("Update failed, no rows changes for seqName: " + seqName);