From 11d56e4c7190c672fb0aff2a2c8fe59771f6dd23 Mon Sep 17 00:00:00 2001 From: Alejandro Revilla Date: Fri, 10 Jun 2016 12:20:59 -0300 Subject: [PATCH] deal with rare race condition in balance cache When we create a balance cache, we lock the account, it is possible for another thread to post a GLTransaction, still uncommitted, that increases the last transaction ID. To prevent this without enforcing expensive locks on every account involving a GLTransaction post, we use a safe window (default to 1000, can be overriden but we believe 1000 is a safe value for use cases like jCard) when deciding the MaxID we use for a bcache entry. --- .../src/main/java/org/jpos/gl/GLSession.java | 14 ++++++++++---- .../src/test/java/org/jpos/gl/BalanceTest.java | 5 +++-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/modules/minigl/src/main/java/org/jpos/gl/GLSession.java b/modules/minigl/src/main/java/org/jpos/gl/GLSession.java index 04ec8ffb69..db5096fc63 100644 --- a/modules/minigl/src/main/java/org/jpos/gl/GLSession.java +++ b/modules/minigl/src/main/java/org/jpos/gl/GLSession.java @@ -45,6 +45,7 @@ public class GLSession { public static final short[] LAYER_ZERO = new short[] { 0 }; public static final BigDecimal ZERO = new BigDecimal ("0.00"); public static final BigDecimal Z = new BigDecimal ("0"); + private long SAFE_WINDOW = 1000L; /** * Construct a GLSession for a given user. @@ -1066,10 +1067,9 @@ else if (acct.isFinalAccount()) { balance[0] = chkp.getBalance(); txnCrit.add (Restrictions.gt ("postDate", chkp.getDate())); } - } else { BalanceCache bcache = getBalanceCache (journal, acct, layersCopy); - if (bcache != null) { + if (bcache != null && bcache.getRef() <= maxId) { balance[0] = bcache.getBalance(); entryCrit.add (Restrictions.gt("id", bcache.getRef())); } @@ -1267,9 +1267,9 @@ else if (acct.isFinalAccount()) { (Journal journal, Account acct, short[] layers) throws HibernateException, GLException { - return createBalanceCache (journal, acct, layers, getMaxGLEntryId()); + return createBalanceCache (journal, acct, layers, getSafeMaxGLEntryId()); } - public BigDecimal createBalanceCache + private BigDecimal createBalanceCache (Journal journal, Account acct, short[] layers, long maxId) throws HibernateException, GLException { @@ -1768,6 +1768,12 @@ private long getMaxGLEntryId () { GLEntry entry = (GLEntry) crit.uniqueResult(); return entry != null ? entry.getId() : 0L; } + private long getSafeMaxGLEntryId() { + return Math.max (getMaxGLEntryId()-SAFE_WINDOW, 0L); + } + public void overrideSafeWindow (long l) { + this.SAFE_WINDOW = l; + } private void recurseChildren (Account acct, List list) { for (Account a : acct.getChildren()) { if (a instanceof FinalAccount) diff --git a/modules/minigl/src/test/java/org/jpos/gl/BalanceTest.java b/modules/minigl/src/test/java/org/jpos/gl/BalanceTest.java index 4d42e0cb5d..a809c48b94 100644 --- a/modules/minigl/src/test/java/org/jpos/gl/BalanceTest.java +++ b/modules/minigl/src/test/java/org/jpos/gl/BalanceTest.java @@ -33,6 +33,7 @@ public class BalanceTest extends TestBase { public void setUp () throws Exception { super.setUp(); + gls.overrideSafeWindow(0L); tj = gls.getJournal ("TestJournal"); cashUS = gls.getFinalAccount ("TestChart", "111"); cashPesos = gls.getAccount ("TestChart", "112"); @@ -58,8 +59,8 @@ public void testBalancesAfterCheckpoint() throws Exception { } public void testBalanceCache() throws Exception { final Transaction tx1 = gls.beginTransaction(); - gls.createBalanceCache (tj, root, GLSession.LAYER_ZERO, 10); - gls.createBalanceCache (tj, root, new short[] { 858 }, 10); + gls.createBalanceCache (tj, root, GLSession.LAYER_ZERO); + gls.createBalanceCache (tj, root, new short[] { 858 }); tx1.commit (); } public void testBalanceCache2() throws Exception {