killbill-memoizeit

analytics: acquire account lock during refreshes Because

10/17/2012 7:10:45 PM

Details

diff --git a/analytics/src/main/java/com/ning/billing/analytics/AnalyticsListener.java b/analytics/src/main/java/com/ning/billing/analytics/AnalyticsListener.java
index bb6961c..8f9a049 100644
--- a/analytics/src/main/java/com/ning/billing/analytics/AnalyticsListener.java
+++ b/analytics/src/main/java/com/ning/billing/analytics/AnalyticsListener.java
@@ -34,6 +34,9 @@ import com.ning.billing.util.callcontext.CallOrigin;
 import com.ning.billing.util.callcontext.InternalCallContext;
 import com.ning.billing.util.callcontext.InternalCallContextFactory;
 import com.ning.billing.util.callcontext.UserType;
+import com.ning.billing.util.globallocker.GlobalLock;
+import com.ning.billing.util.globallocker.GlobalLocker;
+import com.ning.billing.util.globallocker.GlobalLocker.LockerType;
 import com.ning.billing.util.tag.api.ControlTagCreationEvent;
 import com.ning.billing.util.tag.api.ControlTagDefinitionCreationEvent;
 import com.ning.billing.util.tag.api.ControlTagDefinitionDeletionEvent;
@@ -48,12 +51,15 @@ import com.google.inject.Inject;
 
 public class AnalyticsListener {
 
+    private static final int NB_LOCK_TRY = 5;
+
     private final BusinessSubscriptionTransitionDao bstDao;
     private final BusinessAccountDao bacDao;
     private final BusinessInvoiceDao invoiceDao;
     private final BusinessOverdueStatusDao bosDao;
     private final BusinessInvoicePaymentDao bipDao;
     private final BusinessTagDao tagDao;
+    private final GlobalLocker locker;
     private final InternalCallContextFactory internalCallContextFactory;
 
     @Inject
@@ -63,6 +69,7 @@ public class AnalyticsListener {
                              final BusinessOverdueStatusDao bosDao,
                              final BusinessInvoicePaymentDao bipDao,
                              final BusinessTagDao tagDao,
+                             final GlobalLocker locker,
                              final InternalCallContextFactory internalCallContextFactory) {
         this.bstDao = bstDao;
         this.bacDao = bacDao;
@@ -70,6 +77,8 @@ public class AnalyticsListener {
         this.bosDao = bosDao;
         this.bipDao = bipDao;
         this.tagDao = tagDao;
+        // TODO: use accountRecordId when switching to internal events and acquire the lock for all refreshes
+        this.locker = locker;
         this.internalCallContextFactory = internalCallContextFactory;
     }
 
@@ -93,7 +102,15 @@ public class AnalyticsListener {
 
     @Subscribe
     public void handleAccountCreation(final AccountCreationEvent event) {
-        bacDao.accountUpdated(event.getId(), createCallContext(event));
+        GlobalLock lock = null;
+        try {
+            lock = locker.lockWithNumberOfTries(LockerType.ACCOUNT_FOR_ANALYTICS, event.getId().toString(), NB_LOCK_TRY);
+            bacDao.accountUpdated(event.getId(), createCallContext(event));
+        } finally {
+            if (lock != null) {
+                lock.release();
+            }
+        }
     }
 
     @Subscribe
@@ -102,7 +119,15 @@ public class AnalyticsListener {
             return;
         }
 
-        bacDao.accountUpdated(event.getAccountId(), createCallContext(event));
+        GlobalLock lock = null;
+        try {
+            lock = locker.lockWithNumberOfTries(LockerType.ACCOUNT_FOR_ANALYTICS, event.getAccountId().toString(), NB_LOCK_TRY);
+            bacDao.accountUpdated(event.getAccountId(), createCallContext(event));
+        } finally {
+            if (lock != null) {
+                lock.release();
+            }
+        }
     }
 
     @Subscribe
diff --git a/analytics/src/main/java/com/ning/billing/analytics/BusinessAccountDao.java b/analytics/src/main/java/com/ning/billing/analytics/BusinessAccountDao.java
index 56fb66e..2977ace 100644
--- a/analytics/src/main/java/com/ning/billing/analytics/BusinessAccountDao.java
+++ b/analytics/src/main/java/com/ning/billing/analytics/BusinessAccountDao.java
@@ -86,6 +86,7 @@ public class BusinessAccountDao {
     public void updateAccountInTransaction(final BusinessAccount bac, final BusinessAccountSqlDao transactional, final InternalCallContext context) {
         log.info("ACCOUNT UPDATE " + bac);
         transactional.deleteAccount(bac.getAccountId().toString(), context);
+        // Note! There is a window of doom here since we use read committed transactional level by default
         transactional.createAccount(bac, context);
     }
 
diff --git a/invoice/src/main/java/com/ning/billing/invoice/InvoiceDispatcher.java b/invoice/src/main/java/com/ning/billing/invoice/InvoiceDispatcher.java
index bd00de0..50b9127 100644
--- a/invoice/src/main/java/com/ning/billing/invoice/InvoiceDispatcher.java
+++ b/invoice/src/main/java/com/ning/billing/invoice/InvoiceDispatcher.java
@@ -132,7 +132,7 @@ public class InvoiceDispatcher {
                                   final boolean dryRun, final CallContext context) throws InvoiceApiException {
         GlobalLock lock = null;
         try {
-            lock = locker.lockWithNumberOfTries(LockerType.ACCOUNT, accountId.toString(), NB_LOCK_TRY);
+            lock = locker.lockWithNumberOfTries(LockerType.ACCOUNT_FOR_INVOICE_PAYMENTS, accountId.toString(), NB_LOCK_TRY);
 
             return processAccountWithLock(accountId, targetDate, dryRun, context);
         } catch (LockFailedException e) {
diff --git a/payment/src/main/java/com/ning/billing/payment/core/ProcessorBase.java b/payment/src/main/java/com/ning/billing/payment/core/ProcessorBase.java
index d04c1bc..0aa9523 100644
--- a/payment/src/main/java/com/ning/billing/payment/core/ProcessorBase.java
+++ b/payment/src/main/java/com/ning/billing/payment/core/ProcessorBase.java
@@ -167,7 +167,7 @@ public abstract class ProcessorBase {
                 throws PaymentApiException {
             GlobalLock lock = null;
             try {
-                lock = locker.lockWithNumberOfTries(LockerType.ACCOUNT, accountExternalKey, NB_LOCK_TRY);
+                lock = locker.lockWithNumberOfTries(LockerType.ACCOUNT_FOR_INVOICE_PAYMENTS, accountExternalKey, NB_LOCK_TRY);
                 return callback.doOperation();
             } catch (LockFailedException e) {
                 final String format = String.format("Failed to lock account %s", accountExternalKey);
diff --git a/util/src/main/java/com/ning/billing/util/globallocker/GlobalLocker.java b/util/src/main/java/com/ning/billing/util/globallocker/GlobalLocker.java
index 7c59a47..9b1f3d8 100644
--- a/util/src/main/java/com/ning/billing/util/globallocker/GlobalLocker.java
+++ b/util/src/main/java/com/ning/billing/util/globallocker/GlobalLocker.java
@@ -23,7 +23,7 @@ public interface GlobalLocker {
     Boolean isFree(final LockerType service, final String lockKey);
 
     public enum LockerType {
-        // Global ACCOUNT locking
-        ACCOUNT
+        ACCOUNT_FOR_INVOICE_PAYMENTS,
+        ACCOUNT_FOR_ANALYTICS
     }
 }
diff --git a/util/src/test/java/com/ning/billing/util/globallocker/TestMysqlGlobalLocker.java b/util/src/test/java/com/ning/billing/util/globallocker/TestMysqlGlobalLocker.java
index 3604a8f..99b218a 100644
--- a/util/src/test/java/com/ning/billing/util/globallocker/TestMysqlGlobalLocker.java
+++ b/util/src/test/java/com/ning/billing/util/globallocker/TestMysqlGlobalLocker.java
@@ -57,7 +57,7 @@ public class TestMysqlGlobalLocker extends UtilTestSuiteWithEmbeddedDB {
         final String lockName = UUID.randomUUID().toString();
 
         final GlobalLocker locker = new MySqlGlobalLocker(dbi);
-        final GlobalLock lock = locker.lockWithNumberOfTries(LockerType.ACCOUNT, lockName, 3);
+        final GlobalLock lock = locker.lockWithNumberOfTries(LockerType.ACCOUNT_FOR_INVOICE_PAYMENTS, lockName, 3);
 
         dbi.inTransaction(new TransactionCallback<Void>() {
             @Override
@@ -67,11 +67,11 @@ public class TestMysqlGlobalLocker extends UtilTestSuiteWithEmbeddedDB {
                 return null;
             }
         });
-        Assert.assertEquals(locker.isFree(LockerType.ACCOUNT, lockName), Boolean.FALSE);
+        Assert.assertEquals(locker.isFree(LockerType.ACCOUNT_FOR_INVOICE_PAYMENTS, lockName), Boolean.FALSE);
 
         boolean gotException = false;
         try {
-            locker.lockWithNumberOfTries(LockerType.ACCOUNT, lockName, 1);
+            locker.lockWithNumberOfTries(LockerType.ACCOUNT_FOR_INVOICE_PAYMENTS, lockName, 1);
         } catch (LockFailedException e) {
             gotException = true;
         }
@@ -79,7 +79,7 @@ public class TestMysqlGlobalLocker extends UtilTestSuiteWithEmbeddedDB {
 
         lock.release();
 
-        Assert.assertEquals(locker.isFree(LockerType.ACCOUNT, lockName), Boolean.TRUE);
+        Assert.assertEquals(locker.isFree(LockerType.ACCOUNT_FOR_INVOICE_PAYMENTS, lockName), Boolean.TRUE);
     }
 
     public static final class TestMysqlGlobalLockerModule extends AbstractModule {