From c2111b9ccdada7aca5a8cf0ba7ccbc48c0b8011c Mon Sep 17 00:00:00 2001 From: hengsin Date: Thu, 8 Feb 2024 17:20:15 +0800 Subject: [PATCH] IDEMPIERE-6036 Serial number control has potential to generate duplicate serial number under heavy load (#2236) --- .../src/org/compiere/model/MSerNoCtl.java | 74 ++++++++++++++++--- .../src/org/idempiere/test/DictionaryIDs.java | 10 +++ .../test/model/MAttributeSetTest.java | 62 ++++++++++++++++ 3 files changed, 134 insertions(+), 12 deletions(-) diff --git a/org.adempiere.base/src/org/compiere/model/MSerNoCtl.java b/org.adempiere.base/src/org/compiere/model/MSerNoCtl.java index 96f289f35d..6648fffa78 100644 --- a/org.adempiere.base/src/org/compiere/model/MSerNoCtl.java +++ b/org.adempiere.base/src/org/compiere/model/MSerNoCtl.java @@ -17,8 +17,12 @@ package org.compiere.model; import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Savepoint; import java.util.Properties; +import org.adempiere.exceptions.DBException; +import org.compiere.util.Trx; import org.compiere.util.Util; /** @@ -86,18 +90,64 @@ public class MSerNoCtl extends X_M_SerNoCtl */ public String createSerNo () { - StringBuilder name = new StringBuilder(); - if (getPrefix() != null) - name.append(getPrefix()); - int no = getCurrentNext(); - name.append(no); - if (getSuffix() != null) - name.append(getSuffix()); - // - no += getIncrementNo(); - setCurrentNext(no); - saveEx(); - return name.toString(); + //use optimistic locking and try 3 time + set_OptimisticLockingColumns(new String[]{COLUMNNAME_CurrentNext}); + set_UseOptimisticLocking(true); + for(int i = 0; i < 3; i++) + { + this.load(get_TrxName()); + //create savepoint for rollback (if in trx) + Trx trx = null; + Savepoint savepoint = null; + if (get_TrxName() != null) + trx = Trx.get(get_TrxName(), false); + if (trx != null) { + try { + savepoint = trx.setSavepoint(null); + } catch (SQLException e) { + throw new DBException(e); + } + } + try { + StringBuilder name = new StringBuilder(); + if (getPrefix() != null) + name.append(getPrefix()); + int no = getCurrentNext(); + name.append(no); + if (getSuffix() != null) + name.append(getSuffix()); + // + no += getIncrementNo(); + setCurrentNext(no); + saveEx(); + return name.toString(); + } catch (RuntimeException e) { + if (savepoint != null) { + try { + trx.rollback(savepoint); + } catch (SQLException e1) { + throw new DBException(e1); + } + savepoint = null; + } + if (i == 2) + throw e; + //wait 500ms for other trx + try { + Thread.sleep(500); + } catch (InterruptedException e1) { + } + } finally { + if (savepoint != null) { + try { + trx.releaseSavepoint(savepoint); + } catch (SQLException e) { + } + } + } + } + //should never reach here + return null; } // createSerNo } // MSerNoCtl diff --git a/org.idempiere.test/src/org/idempiere/test/DictionaryIDs.java b/org.idempiere.test/src/org/idempiere/test/DictionaryIDs.java index fd143db2bc..ce180f06d5 100644 --- a/org.idempiere.test/src/org/idempiere/test/DictionaryIDs.java +++ b/org.idempiere.test/src/org/idempiere/test/DictionaryIDs.java @@ -553,6 +553,16 @@ public final class DictionaryIDs { } } + public enum M_SerNoCtl { + SERIAL_NO_EXAMPLE(100); + + public final int id; + + private M_SerNoCtl(int id) { + this.id = id; + } + } + public enum M_Shipper { UPS(100), FERTILIZER_INTERNAL_SHIPPER(50001), diff --git a/org.idempiere.test/src/org/idempiere/test/model/MAttributeSetTest.java b/org.idempiere.test/src/org/idempiere/test/model/MAttributeSetTest.java index c7870a32bd..2f0567683d 100644 --- a/org.idempiere.test/src/org/idempiere/test/model/MAttributeSetTest.java +++ b/org.idempiere.test/src/org/idempiere/test/model/MAttributeSetTest.java @@ -26,13 +26,19 @@ package org.idempiere.test.model; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.concurrent.atomic.AtomicReference; + import org.compiere.model.MAttribute; import org.compiere.model.MAttributeSet; +import org.compiere.model.MAttributeSetInstance; import org.compiere.model.MAttributeUse; import org.compiere.util.Env; +import org.compiere.util.Trx; +import org.compiere.util.TrxRunnable; import org.idempiere.test.AbstractTestCase; import org.idempiere.test.DictionaryIDs; import org.junit.jupiter.api.Test; @@ -115,4 +121,60 @@ public class MAttributeSetTest extends AbstractTestCase { as.load(getTrxName()); assertTrue(as.isInstanceAttribute()); } + + @Test + public void testGenerateUniqueSerial() { + MAttributeSet mas = new MAttributeSet(Env.getCtx(), DictionaryIDs.M_AttributeSet.PATIO_CHAIR.id, null); + mas.setM_SerNoCtl_ID(DictionaryIDs.M_SerNoCtl.SERIAL_NO_EXAMPLE.id); + try { + mas.saveEx(); + Trx trx1 = Trx.get(Trx.createTrxName(), true); + Trx trx2 = Trx.get(Trx.createTrxName(), true); + AtomicReferenceatomic1 = new AtomicReference(null); + AtomicReferenceatomic2 = new AtomicReference(null); + try { + TrxRunnable runnable1 = (trxName -> { + MAttributeSetInstance asi1 = new MAttributeSetInstance(Env.getCtx(), 0, mas.get_ID(), trxName); + String serno1 = asi1.getSerNo(true); + try { + Thread.sleep(500); + } catch (InterruptedException e) { + } + Trx.get(trxName, false).commit(); + atomic1.set(serno1); + }); + TrxRunnable runnable2 = (trxName -> { + MAttributeSetInstance asi2 = new MAttributeSetInstance(Env.getCtx(), 0, mas.get_ID(), trx2.getTrxName()); + String serno2 = asi2.getSerNo(true); + Trx.get(trxName, false).commit(); + atomic2.set(serno2); + }); + Thread t1 = new Thread(() -> { + Trx.run(trx1.getTrxName(), runnable1); + }) ; + Thread t2 = new Thread(() -> { + Trx.run(trx2.getTrxName(), runnable2); + }); + t1.start(); + t2.start(); + try { + t1.join(); + } catch (InterruptedException e) { + } + try { + t2.join(); + } catch (InterruptedException e) { + } + assertNotNull(atomic1.get(), "Serial number 1 not generated"); + assertNotNull(atomic2.get(), "Serial number 2 not generated"); + assertNotEquals(atomic1.get(), atomic2.get(), "Duplicate serial number generated"); + } finally { + trx1.close(); + trx2.close(); + } + } finally { + mas.setM_SerNoCtl_ID(0); + mas.saveEx(); + } + } }