[HHH-3817] Further refine, test PutFromLoadValidator

git-svn-id: https://svn.jboss.org/repos/hibernate/core/trunk@17657 1b8cb986-b30d-0410-93ca-fae66ebed9b2

cache-jbosscache/src/main/java/org/hibernate/cache/jbc/access/PutFromLoadValidator.java

@@ -115,11 +115,14 @@ public class PutFromLoadValidator
     *                           may be <code>null</code>
     */
    public PutFromLoadValidator(TransactionManager transactionManager) {
-      this(transactionManager, NAKED_PUT_INVALIDATION_PERIOD, PENDING_PUT_OVERAGE_PERIOD, PENDING_PUT_RECENT_PERIOD, MAX_PENDING_PUT_DELAY);
+      this(transactionManager, NAKED_PUT_INVALIDATION_PERIOD, PENDING_PUT_OVERAGE_PERIOD, 
+    		  PENDING_PUT_RECENT_PERIOD, MAX_PENDING_PUT_DELAY);
    }
    
-   /** Constructor variant for use by unit tests; allows control of variouts timeouts by the test. */
+   /** Constructor variant for use by unit tests; allows control of various timeouts by the test. */
-   protected PutFromLoadValidator(TransactionManager transactionManager, long nakedPutInvalidationPeriod, long pendingPutOveragePeriod, long pendingPutRecentPeriod, long maxPendingPutDelay) {
+   protected PutFromLoadValidator(TransactionManager transactionManager, 
+		   long nakedPutInvalidationPeriod, long pendingPutOveragePeriod, 
+		   long pendingPutRecentPeriod, long maxPendingPutDelay) {
       this.transactionManager = transactionManager;
       this.nakedPutInvalidationPeriod = nakedPutInvalidationPeriod;
       this.pendingPutOveragePeriod = pendingPutOveragePeriod;
@@ -127,6 +130,8 @@ public class PutFromLoadValidator
       this.maxPendingPutDelay = maxPendingPutDelay;
    }
 
+   // -----------------------------------------------------------------  Public
+   
    public boolean isPutValid(Object key)
    {
       boolean valid = false;
@@ -264,6 +269,41 @@ public class PutFromLoadValidator
       // Guard against memory leaks
       preventOutdatedPendingPuts(pendingPut);
    }
+   // --------------------------------------------------------------  Protected
+   
+   /** Only for use by unit tests; may be removed at any time */
+   protected int getPendingPutQueueLength() {
+	   pendingLock.lock();
+	   try {
+		   return pendingQueue.size();
+	   }
+	   finally {
+		   pendingLock.unlock();
+	   }
+   }
+   
+   /** Only for use by unit tests; may be removed at any time */
+   protected int getOveragePendingPutQueueLength() {
+	   pendingLock.lock();
+	   try {
+		   return overagePendingQueue.size();
+	   }
+	   finally {
+		   pendingLock.unlock();
+	   }
+   }
+   
+   /** Only for use by unit tests; may be removed at any time */
+   protected int getRemovalQueueLength() {
+	   removalsLock.lock();
+	   try {
+		   return removalsQueue.size();
+	   }
+	   finally {
+		   removalsLock.unlock();
+	   }
+   }
+   // ----------------------------------------------------------------  Private
 
    private Object getOwnerForPut()
    {
@@ -307,7 +347,7 @@ public class PutFromLoadValidator
          
          int pos = 0;
          while (pendingQueue.size() > pos) {
-            WeakReference<PendingPut> ref = pendingQueue.get(0);
+            WeakReference<PendingPut> ref = pendingQueue.get(pos);
             PendingPut item = ref.get();
             if (item == null || item.completed) {
                pendingQueue.remove(pos);
@@ -343,6 +383,7 @@ public class PutFromLoadValidator
             }
             else {
                if (item.timestamp < mustCleanTime) {
+                  overagePendingQueue.remove(0);
                   toClean = item;
                }
                break;

cache-jbosscache/src/test/java/org/hibernate/test/cache/jbc/access/PutFromLoadValidatorUnitTestCase.java

@@ -23,13 +23,20 @@
  */
 package org.hibernate.test.cache.jbc.access;
 
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import javax.transaction.Transaction;
 import javax.transaction.TransactionManager;
 
+import junit.framework.TestCase;
+
 import org.hibernate.cache.jbc.access.PutFromLoadValidator;
 import org.hibernate.test.cache.jbc.functional.util.DualNodeJtaTransactionManagerImpl;
 
-import junit.framework.TestCase;
-
 /**
  * Tests of {@link PutFromLoadValidator}.
  * 
@@ -37,8 +44,7 @@ import junit.framework.TestCase;
  * 
  * @version $Revision: $
  */
-public class PutFromLoadValidatorUnitTestCase extends TestCase
+public class PutFromLoadValidatorUnitTestCase extends TestCase {
-{
 	private Object KEY1 = "KEY1";
 
 	private TransactionManager tm;
@@ -48,24 +54,20 @@ public class PutFromLoadValidatorUnitTestCase extends TestCase
 	}
 
 	@Override
-   protected void setUp() throws Exception
+	protected void setUp() throws Exception {
-   {
 		super.setUp();
 		tm = DualNodeJtaTransactionManagerImpl.getInstance("test");
 	}
 
 	@Override
-   protected void tearDown() throws Exception
+	protected void tearDown() throws Exception {
-   {
 		try {
 			super.tearDown();
-      }
+		} finally {
-      finally {
 			tm = null;
 			try {
 				DualNodeJtaTransactionManagerImpl.cleanupTransactions();
-         }
+			} finally {
-         finally {
 				DualNodeJtaTransactionManagerImpl.cleanupTransactionManagers();
 			}
 		}
@@ -80,7 +82,8 @@ public class PutFromLoadValidatorUnitTestCase extends TestCase
 	}
 
 	private void nakedPutTest(boolean transactional) throws Exception {
-      PutFromLoadValidator testee = new PutFromLoadValidator(transactional ? tm : null);
+		PutFromLoadValidator testee = new PutFromLoadValidator(
+				transactional ? tm : null);
 		if (transactional) {
 			tm.begin();
 		}
@@ -96,7 +99,8 @@ public class PutFromLoadValidatorUnitTestCase extends TestCase
 	}
 
 	private void registeredPutTest(boolean transactional) throws Exception {
-      PutFromLoadValidator testee = new PutFromLoadValidator(transactional ? tm : null);
+		PutFromLoadValidator testee = new PutFromLoadValidator(
+				transactional ? tm : null);
 		if (transactional) {
 			tm.begin();
 		}
@@ -120,13 +124,13 @@ public class PutFromLoadValidatorUnitTestCase extends TestCase
 		nakedPutAfterRemovalTest(true, true);
 	}
 
-   private void nakedPutAfterRemovalTest(boolean transactional, boolean removeRegion) throws Exception
+	private void nakedPutAfterRemovalTest(boolean transactional,
-   {
+			boolean removeRegion) throws Exception {
-      PutFromLoadValidator testee = new PutFromLoadValidator(transactional ? tm : null);
+		PutFromLoadValidator testee = new PutFromLoadValidator(
+				transactional ? tm : null);
 		if (removeRegion) {
 			testee.regionRemoved();
-      }
+		} else {
-      else {
 			testee.keyRemoved(KEY1);
 		}
 		if (transactional) {
@@ -140,7 +144,8 @@ public class PutFromLoadValidatorUnitTestCase extends TestCase
 		registeredPutAfterRemovalTest(false, false);
 	}
 
-   public void testRegisteredPutAfterKeyRemovalTransactional() throws Exception {
+	public void testRegisteredPutAfterKeyRemovalTransactional()
+			throws Exception {
 		registeredPutAfterRemovalTest(true, false);
 	}
 
@@ -148,17 +153,18 @@ public class PutFromLoadValidatorUnitTestCase extends TestCase
 		registeredPutAfterRemovalTest(false, true);
 	}
 
-   public void testRegisteredPutAfterRegionRemovalTransactional() throws Exception {
+	public void testRegisteredPutAfterRegionRemovalTransactional()
+			throws Exception {
 		registeredPutAfterRemovalTest(true, true);
 	}
 
-   private void registeredPutAfterRemovalTest(boolean transactional, boolean removeRegion) throws Exception
+	private void registeredPutAfterRemovalTest(boolean transactional,
-   {
+			boolean removeRegion) throws Exception {
-      PutFromLoadValidator testee = new PutFromLoadValidator(transactional ? tm : null);
+		PutFromLoadValidator testee = new PutFromLoadValidator(
+				transactional ? tm : null);
 		if (removeRegion) {
 			testee.regionRemoved();
-      }
+		} else {
-      else {
 			testee.keyRemoved(KEY1);
 		}
 		if (transactional) {
@@ -172,29 +178,32 @@ public class PutFromLoadValidatorUnitTestCase extends TestCase
 		registeredPutWithInterveningRemovalTest(false, false);
 	}
 
-   public void testRegisteredPutWithInterveningKeyRemovalTransactional() throws Exception {
+	public void testRegisteredPutWithInterveningKeyRemovalTransactional()
+			throws Exception {
 		registeredPutWithInterveningRemovalTest(true, false);
 	}
 
-   public void testRegisteredPutWithInterveningRegionRemoval() throws Exception {
+	public void testRegisteredPutWithInterveningRegionRemoval()
+			throws Exception {
 		registeredPutWithInterveningRemovalTest(false, true);
 	}
 
-   public void testRegisteredPutWithInterveningRegionRemovalTransactional() throws Exception {
+	public void testRegisteredPutWithInterveningRegionRemovalTransactional()
+			throws Exception {
 		registeredPutWithInterveningRemovalTest(true, true);
 	}
 
-   private void registeredPutWithInterveningRemovalTest(boolean transactional, boolean removeRegion) throws Exception
+	private void registeredPutWithInterveningRemovalTest(boolean transactional,
-   {
+			boolean removeRegion) throws Exception {
-      PutFromLoadValidator testee = new PutFromLoadValidator(transactional ? tm : null);
+		PutFromLoadValidator testee = new PutFromLoadValidator(
+				transactional ? tm : null);
 		if (transactional) {
 			tm.begin();
 		}
 		testee.registerPendingPut(KEY1);
 		if (removeRegion) {
 			testee.regionRemoved();
-      }
+		} else {
-      else {
 			testee.keyRemoved(KEY1);
 		}
 		assertFalse(testee.isPutValid(KEY1));
@@ -204,7 +213,8 @@ public class PutFromLoadValidatorUnitTestCase extends TestCase
 		delayedNakedPutAfterRemovalTest(false, false);
 	}
 
-   public void testDelayedNakedPutAfterKeyRemovalTransactional() throws Exception {
+	public void testDelayedNakedPutAfterKeyRemovalTransactional()
+			throws Exception {
 		delayedNakedPutAfterRemovalTest(true, false);
 	}
 
@@ -212,17 +222,18 @@ public class PutFromLoadValidatorUnitTestCase extends TestCase
 		delayedNakedPutAfterRemovalTest(false, true);
 	}
 
-   public void testDelayedNakedPutAfterRegionRemovalTransactional() throws Exception {
+	public void testDelayedNakedPutAfterRegionRemovalTransactional()
+			throws Exception {
 		delayedNakedPutAfterRemovalTest(true, true);
 	}
 
-   private void delayedNakedPutAfterRemovalTest(boolean transactional, boolean removeRegion) throws Exception
+	private void delayedNakedPutAfterRemovalTest(boolean transactional,
-   {
+			boolean removeRegion) throws Exception {
-      PutFromLoadValidator testee = new TestValidator(transactional ? tm : null, 100, 1000, 500, 10000);
+		PutFromLoadValidator testee = new TestValidator(transactional ? tm
+				: null, 100, 1000, 500, 10000);
 		if (removeRegion) {
 			testee.regionRemoved();
-      }
+		} else {
-      else {
 			testee.keyRemoved(KEY1);
 		}
 		if (transactional) {
@@ -233,14 +244,185 @@ public class PutFromLoadValidatorUnitTestCase extends TestCase
 
 	}
 	
+	public void testMultipleRegistrations() throws Exception {
+		multipleRegistrationtest(false);
+	}
+	
+	public void testMultipleRegistrationsTransactional() throws Exception {
+		multipleRegistrationtest(true);
+	}
+
+	private void multipleRegistrationtest(final boolean transactional) throws Exception {
+		final PutFromLoadValidator testee = new PutFromLoadValidator(transactional ? tm : null);
+		
+		final CountDownLatch registeredLatch = new CountDownLatch(3);
+		final CountDownLatch finishedLatch = new CountDownLatch(3);
+		final AtomicInteger success = new AtomicInteger();
+		
+		Runnable r = new Runnable() {
+			public void run() {
+				try {
+					if (transactional) {
+						tm.begin();
+					}
+					testee.registerPendingPut(KEY1);
+					registeredLatch.countDown();
+					registeredLatch.await(5, TimeUnit.SECONDS);
+					if (testee.isPutValid(KEY1)) {
+						success.incrementAndGet();
+					}
+					finishedLatch.countDown();
+				}
+				catch (Exception e)  {
+					e.printStackTrace();
+				}
+			}
+		};
+		
+		ExecutorService executor = Executors.newFixedThreadPool(3);
+		
+		// Start with a removal so the "isPutValid" calls will fail if
+		// any of the concurrent activity isn't handled properly
+		
+		testee.regionRemoved();
+		
+		// Do the registration + isPutValid calls
+		executor.execute(r);
+		executor.execute(r);
+		executor.execute(r);
+		
+		finishedLatch.await(5, TimeUnit.SECONDS);
+		
+		assertEquals("All threads succeeded", 3, success.get());
+	}
+	
+	/**
+	 * White box test for ensuring key removals get cleaned up.
+	 * 
+	 * @throws Exception
+	 */
+	public void testRemovalCleanup() throws Exception {
+		TestValidator testee = new TestValidator(null, 200, 1000, 500, 10000);
+		testee.keyRemoved("KEY1");
+		testee.keyRemoved("KEY2");
+		Thread.sleep(210);
+		assertEquals(2, testee.getRemovalQueueLength());
+		testee.keyRemoved("KEY1");
+		assertEquals(2, testee.getRemovalQueueLength());
+		testee.keyRemoved("KEY2");
+		assertEquals(2, testee.getRemovalQueueLength());
+	}
+	
+	/** 
+	 * Very much a white box test of the logic for ensuring pending
+	 * put registrations get cleaned up.
+	 *  
+	 * @throws Exception
+	 */
+	public void testPendingPutCleanup() throws Exception {
+		TestValidator testee = new TestValidator(tm, 5000, 600, 300, 900);
+		
+		// Start with a regionRemoval so we can confirm at the end that all
+		// registrations have been cleaned out
+		testee.regionRemoved();
+		
+		testee.registerPendingPut("1");
+		testee.registerPendingPut("2");
+		testee.registerPendingPut("3");
+		testee.registerPendingPut("4");
+		testee.registerPendingPut("5");
+		testee.registerPendingPut("6");
+		testee.isPutValid("6");
+		testee.isPutValid("2");
+        // ppq = [1,2(c),3,4,5,6(c)]
+		assertEquals(6, testee.getPendingPutQueueLength());
+		assertEquals(0, testee.getOveragePendingPutQueueLength());
+		
+		// Sleep past "pendingPutRecentPeriod"
+		Thread.sleep(310);
+		testee.registerPendingPut("7");
+        // White box -- should have cleaned out 2 (completed) but 
+		// not gotten to 6 (also removed)
+		// ppq = [1,3,4,5,6(c),7]
+		assertEquals(0, testee.getOveragePendingPutQueueLength());
+		assertEquals(6, testee.getPendingPutQueueLength());
+		
+		// Sleep past "pendingPutOveragePeriod"
+		Thread.sleep(310);
+		testee.registerPendingPut("8");
+		// White box -- should have cleaned out 6 (completed) and 
+		// moved 1, 3, 4  and 5 to overage queue
+		// oppq = [1,3,4,5] ppq = [7,8]
+        assertEquals(4, testee.getOveragePendingPutQueueLength());
+		assertEquals(2, testee.getPendingPutQueueLength());
+		
+		// Sleep past "maxPendingPutDelay"
+		Thread.sleep(310);
+		testee.isPutValid("3");
+		// White box -- should have cleaned out 1 (overage) and 
+		// moved 7 to overage queue
+		// oppq = [3(c),4,5,7] ppq=[8]
+        assertEquals(4, testee.getOveragePendingPutQueueLength());
+		assertEquals(1, testee.getPendingPutQueueLength());
+		
+		// Sleep past "maxPendingPutDelay"
+		Thread.sleep(310);
+		tm.begin();
+		testee.registerPendingPut("7");
+		Transaction tx = tm.suspend();
+		
+		// White box -- should have cleaned out 3 (completed) 
+		// and 4 (overage) and moved 8 to overage queue
+		// We now have 5,7,8 in overage and 7tx in pending
+		// oppq = [5,7,8] ppq=[7tx]
+        assertEquals(3, testee.getOveragePendingPutQueueLength());
+		assertEquals(1, testee.getPendingPutQueueLength());
+		
+		// Validate that only expected items can do puts, thus indirectly
+		// proving the others have been cleaned out of pendingPuts map
+		assertFalse(testee.isPutValid("1"));
+		// 5 was overage, so should have been cleaned
+		assertEquals(2, testee.getOveragePendingPutQueueLength());
+		assertFalse(testee.isPutValid("2"));
+		// 7 was overage, so should have been cleaned
+		assertEquals(1, testee.getOveragePendingPutQueueLength());
+		assertFalse(testee.isPutValid("3"));
+		assertFalse(testee.isPutValid("4"));
+		assertFalse(testee.isPutValid("5"));
+		assertFalse(testee.isPutValid("6"));
+		assertFalse(testee.isPutValid("7"));
+		assertTrue(testee.isPutValid("8"));
+		tm.resume(tx);
+		assertTrue(testee.isPutValid("7"));
+	}
+
 	private static class TestValidator extends PutFromLoadValidator {
 
-      protected TestValidator(TransactionManager transactionManager, long nakedPutInvalidationPeriod,
+		protected TestValidator(TransactionManager transactionManager,
-            long pendingPutOveragePeriod, long pendingPutRecentPeriod, long maxPendingPutDelay)
+				long nakedPutInvalidationPeriod, long pendingPutOveragePeriod,
-      {
+				long pendingPutRecentPeriod, long maxPendingPutDelay) {
-         super(transactionManager, nakedPutInvalidationPeriod, pendingPutOveragePeriod, pendingPutRecentPeriod,
+			super(transactionManager, nakedPutInvalidationPeriod,
+					pendingPutOveragePeriod, pendingPutRecentPeriod,
 					maxPendingPutDelay);
 		}
 
+		@Override
+		public int getOveragePendingPutQueueLength() {
+			// TODO Auto-generated method stub
+			return super.getOveragePendingPutQueueLength();
+		}
+
+		@Override
+		public int getPendingPutQueueLength() {
+			// TODO Auto-generated method stub
+			return super.getPendingPutQueueLength();
+		}
+
+		@Override
+		public int getRemovalQueueLength() {
+			// TODO Auto-generated method stub
+			return super.getRemovalQueueLength();
+		}
+
 	}
 }