You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2011/07/09 00:58:35 UTC
svn commit: r1144537 - in /ofbiz/trunk/framework:
common/config/general.properties entity/src/org/ofbiz/entity/Delegator.java
entity/src/org/ofbiz/entity/GenericDelegator.java
entity/src/org/ofbiz/entity/util/SequenceUtil.java
Author: jleroux
Date: Fri Jul 8 22:58:35 2011
New Revision: 1144537
URL: http://svn.apache.org/viewvc?rev=1144537&view=rev
Log:
Closes "SequenceUtil may generate duplicate IDs in Load Balancing mode" https://issues.apache.org/jira/browse/OFBIZ-2353
Pb: If OFBiz is deployed 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.
The OFBIZ-2353 SELECT FOR UPDATE solution.patch provides a simple mean to quickly resolve this issue. I did not remove the bank loop (useless since there is a DB contention).
I created a cluster general property. But since, most of the time, when you use a cluster you use also a sole database with a delegator having distributed-cache-clear-enabled set to true, I retrieve this information to automatically use SELECT FOR UPDATE in this case
This has been tested on a cluster with JMeter. BTW I found that UserLoginHistory can throw errors because it does not use a SeqId in PK but only login+fromDate. I guess there are other cases like that but in reality it's unlikely that someone log on 2 diff machines with the same login at the same time (ms), just a JMeter side effect...
Modified:
ofbiz/trunk/framework/common/config/general.properties
ofbiz/trunk/framework/entity/src/org/ofbiz/entity/Delegator.java
ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/SequenceUtil.java
Modified: ofbiz/trunk/framework/common/config/general.properties
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/config/general.properties?rev=1144537&r1=1144536&r2=1144537&view=diff
==============================================================================
--- ofbiz/trunk/framework/common/config/general.properties (original)
+++ ofbiz/trunk/framework/common/config/general.properties Fri Jul 8 22:58:35 2011
@@ -130,3 +130,6 @@ http.localhost=ABQIAAAAtt0d8djaYFkk8N5LJ
# -- Y if you want to display the multi-tenant textbox in the login page
multitenant=N
+
+# -- Y if you use a cluster
+cluster=N
Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/Delegator.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/Delegator.java?rev=1144537&r1=1144536&r2=1144537&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/Delegator.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/Delegator.java Fri Jul 8 22:58:35 2011
@@ -1222,4 +1222,10 @@ public interface Delegator {
*/
public boolean getEnabledJMS();
+ /**
+ * Get use of Distributed Cache Clear mechanism status
+ * @return boolean true if this delegator uses a Distributed Cache Clear mechanism
+ */
+ public boolean useDistributedCacheClear();
+
}
Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java?rev=1144537&r1=1144536&r2=1144537&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java Fri Jul 8 22:58:35 2011
@@ -2468,7 +2468,7 @@ public class GenericDelegator implements
synchronized (this) {
if (sequencer == null) {
ModelEntity seqEntity = this.getModelEntity("SequenceValueItem");
- sequencer = new SequenceUtil(this.getEntityHelperInfo("SequenceValueItem"), seqEntity, "seqName", "seqId");
+ sequencer = new SequenceUtil(this, this.getEntityHelperInfo("SequenceValueItem"), seqEntity, "seqName", "seqId");
}
}
}
@@ -2918,4 +2918,12 @@ public class GenericDelegator implements
public boolean getEnabledJMS() {
return this.enableJMS;
}
+
+ /* (non-Javadoc)
+ * @see org.ofbiz.entity.Delegator#getEnableJMS()
+ */
+ public boolean useDistributedCacheClear() {
+ return this.getDelegatorInfo().useDistributedCacheClear;
+ }
+
}
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=1144537&r1=1144536&r2=1144537&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 Fri Jul 8 22:58:35 2011
@@ -28,6 +28,8 @@ import java.util.Map;
import javax.transaction.Transaction;
import org.ofbiz.base.util.Debug;
+import org.ofbiz.base.util.UtilProperties;
+import org.ofbiz.entity.GenericDelegator;
import org.ofbiz.entity.GenericEntityException;
import org.ofbiz.entity.datasource.GenericHelperInfo;
import org.ofbiz.entity.jdbc.ConnectionFactory;
@@ -50,8 +52,9 @@ public class SequenceUtil {
private final String tableName;
private final String nameColName;
private final String idColName;
+ private final boolean clustered;
- public SequenceUtil(GenericHelperInfo helperInfo, ModelEntity seqEntity, String nameFieldName, String idFieldName) {
+ public SequenceUtil(GenericDelegator delegator, GenericHelperInfo helperInfo, ModelEntity seqEntity, String nameFieldName, String idFieldName) {
this.helperInfo = helperInfo;
if (seqEntity == null) {
throw new IllegalArgumentException("The sequence model entity was null but is required.");
@@ -76,6 +79,7 @@ public class SequenceUtil {
bankSize = seqEntity.getSequenceBankSize().longValue();
}
this.bankSize = bankSize;
+ clustered = delegator.useDistributedCacheClear() || "Y".equals(UtilProperties.getPropertyValue("general.properties", "clustered"));
}
public Long getNextSeqId(String seqName, long staggerMax, ModelEntity seqModelEntity) {
@@ -182,8 +186,8 @@ public class SequenceUtil {
this.seqName + "; start of loop val1=" + val1 + ", val2=" + val2 + ", bankSize=" + bankSize, module);
// not sure if this synchronized block is totally necessary, the method is synchronized but it does do a wait/sleep
- //outside of this block, and this is the really sensitive block, so making sure it is isolated; there is some overhead
- //to this, but very bad things can happen if we try to do too many of these at once for a single sequencer
+ // outside of this block, and this is the really sensitive block, so making sure it is isolated; there is some overhead
+ // to this, but very bad things can happen if we try to do too many of these at once for a single sequencer
synchronized (this) {
Transaction suspendedTransaction = null;
try {
@@ -218,8 +222,11 @@ public class SequenceUtil {
// we shouldn't need this, and some TX managers complain about it, so not including it: connection.setAutoCommit(false);
stmt = connection.createStatement();
-
- sql = "SELECT " + SequenceUtil.this.idColName + " FROM " + SequenceUtil.this.tableName + " WHERE " + SequenceUtil.this.nameColName + "='" + this.seqName + "'";
+ if (clustered) {
+ sql = "SELECT " + SequenceUtil.this.idColName + " FROM " + SequenceUtil.this.tableName + " WHERE " + SequenceUtil.this.nameColName + "='" + this.seqName + "'" + " FOR UPDATE";
+ } else {
+ sql = "SELECT " + SequenceUtil.this.idColName + " FROM " + SequenceUtil.this.tableName + " WHERE " + SequenceUtil.this.nameColName + "='" + this.seqName + "'";
+ }
rs = stmt.executeQuery(sql);
boolean gotVal1 = false;
if (rs.next()) {
@@ -241,8 +248,12 @@ public class SequenceUtil {
if (stmt.executeUpdate(sql) <= 0) {
throw new GenericEntityException("[SequenceUtil.SequenceBank.fillBank] update failed, no rows changes for seqName: " + seqName);
}
+ if (clustered) {
+ sql = "SELECT " + SequenceUtil.this.idColName + " FROM " + SequenceUtil.this.tableName + " WHERE " + SequenceUtil.this.nameColName + "='" + this.seqName + "'" + " FOR UPDATE";
- sql = "SELECT " + SequenceUtil.this.idColName + " FROM " + SequenceUtil.this.tableName + " WHERE " + SequenceUtil.this.nameColName + "='" + this.seqName + "'";
+ } else {
+ sql = "SELECT " + SequenceUtil.this.idColName + " FROM " + SequenceUtil.this.tableName + " WHERE " + SequenceUtil.this.nameColName + "='" + this.seqName + "'";
+ }
rs = stmt.executeQuery(sql);
boolean gotVal2 = false;
if (rs.next()) {