REPO-3493: node cleanup consistent query (#139)

src/main/java/org/alfresco/repo/node/db/DeletedNodeCleanupWorker.java

@@ -43,6 +43,8 @@ import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransacti
 public class DeletedNodeCleanupWorker extends AbstractNodeCleanupWorker
 {
     private long minPurgeAgeMs;
+    // used for tests, to consider only transactions after a certain commit time
+    private long fromCustomCommitTime = -1;
     
     // Unused transactions will be purged in chunks determined by commit time boundaries. 'index.tracking.purgeSize' specifies the size
     // of the chunk (in ms). Default is a couple of hours.
@@ -88,6 +90,16 @@ public class DeletedNodeCleanupWorker extends AbstractNodeCleanupWorker
         this.minPurgeAgeMs = ((long) minPurgeAgeDays) * 24L * 3600L * 1000L;
     }
 
+    /**
+     * Set a custom "from commit time" that will consider only the transactions after this specified time
+     * Setting a negative value or 0 will trigger the default behaviour to get the oldest "from time" for any deleted node
+     *
+     * @param fromCustomCommitTime the custom from commit time value
+     */
+    public void setFromCustomCommitTime(long fromCustomCommitTime)
+    {
+        this.fromCustomCommitTime = fromCustomCommitTime;
+    }
     /**
      * Set the purge transaction block size. This determines how many unused transactions are purged in one go.
      * 
@@ -109,8 +121,11 @@ public class DeletedNodeCleanupWorker extends AbstractNodeCleanupWorker
         final List<String> results = new ArrayList<String>(100);
 
         final long maxCommitTime = System.currentTimeMillis() - minAge;
-        long fromCommitTime = nodeDAO.getMinTxnCommitTimeForDeletedNodes().longValue();
+        long fromCommitTime = fromCustomCommitTime;
-        
+        if (fromCommitTime <= 0L)
+        {
+            fromCommitTime = nodeDAO.getMinTxnCommitTimeForDeletedNodes().longValue();
+        }
         if ( fromCommitTime == 0L )
         {
               String msg = "There are no old nodes to purge.";
@@ -225,8 +240,11 @@ public class DeletedNodeCleanupWorker extends AbstractNodeCleanupWorker
         final List<String> results = new ArrayList<String>(100);
 
         final long maxCommitTime = System.currentTimeMillis() - minAge;
-    	long fromCommitTime = nodeDAO.getMinUnusedTxnCommitTime().longValue();
+        long fromCommitTime = fromCustomCommitTime;
-
+        if (fromCommitTime <= 0L)
+        {
+            fromCommitTime = nodeDAO.getMinUnusedTxnCommitTime().longValue();
+        }
     	// delete unused transactions in batches of size 'purgeTxnBlockSize'
         while (true)
         {

src/main/resources/alfresco/ibatis/org.alfresco.repo.domain.dialect.Dialect/node-delete-SqlMap.xml

@@ -9,9 +9,9 @@
         delete from alf_node
         where
             type_qname_id = #{typeQNameId} and
-            transaction_id <=
+            transaction_id IN
             (
-                select max(txn.id) from alf_transaction txn
+                select txn.id from alf_transaction txn
                 where
                     txn.commit_time_ms >= #{minCommitTime} and
                     txn.commit_time_ms < #{maxCommitTime}

src/test/java/org/alfresco/AllDBTestsTestSuite.java

@@ -79,6 +79,8 @@ import org.junit.runners.Suite;
     // REPO-2963 : Tests causing a cascade of failures in AllDBTestsTestSuite on PostgreSQL/MySQL	
     // Moved at the bottom of the suite because DbNodeServiceImplTest.testNodeCleanupRegistry() takes a long time on a clean DB.
     org.alfresco.repo.node.db.DbNodeServiceImplTest.class,
+
+    org.alfresco.repo.node.cleanup.TransactionCleanupTest.class
 })
 public class AllDBTestsTestSuite
 {

src/test/java/org/alfresco/repo/node/cleanup/TransactionCleanupTest.java

@@ -57,7 +57,7 @@ import org.alfresco.service.namespace.QName;
 import org.alfresco.service.transaction.TransactionService;
 import org.alfresco.test_category.OwnJVMTestsCategory;
 import org.alfresco.util.ApplicationContextHelper;
-import org.alfresco.util.testing.category.LuceneTests;
+import org.alfresco.util.testing.category.DBTests;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.junit.After;
@@ -71,12 +71,12 @@ import org.springframework.extensions.webscripts.GUID;
  * @author Derek Hulley
  * @since 4.0
  */
-@Category({OwnJVMTestsCategory.class, LuceneTests.class})
+@Category({OwnJVMTestsCategory.class, DBTests.class })
 public class TransactionCleanupTest
 {
     private static Log logger = LogFactory.getLog(TransactionCleanupTest.class);
     
-    private static ApplicationContext ctx = ApplicationContextHelper.getApplicationContext();
+    private ApplicationContext ctx = ApplicationContextHelper.getApplicationContext();
     
     private TransactionService transactionService;
     private NodeService nodeService;
@@ -93,6 +93,8 @@ public class TransactionCleanupTest
     private NodeRef nodeRef5;
     private RetryingTransactionHelper helper;
 
+    private long fromCustomCommitTime = -1;
+
     @SuppressWarnings("unchecked")
     @Before
     public void before()
@@ -176,6 +178,34 @@ public class TransactionCleanupTest
 		return txnIds;
     }
 
+    private Map<NodeRef, List<String>> createTransactionsForNodePurgeTest()
+    {
+        Map<NodeRef, List<String>> txnIds = new HashMap<NodeRef, List<String>>();
+        DeleteNode deleteNode4 = new DeleteNode(nodeRef4);
+        DeleteNode deleteNode5 = new DeleteNode(nodeRef5);
+        List<String> txnIds4 = new ArrayList<String>();
+        List<String> txnIds5 = new ArrayList<String>();
+        txnIds.put(nodeRef4, txnIds4);
+        txnIds.put(nodeRef5, txnIds5);
+
+        String txnId4 = helper.doInTransaction(deleteNode4, false, true);
+        txnIds4.add(txnId4);
+        try
+        {
+            Thread.sleep(500);
+        }
+        catch (Exception e)
+        {
+            // not a problem
+        }
+        fromCustomCommitTime = System.currentTimeMillis();
+
+        String txnId5 = helper.doInTransaction(deleteNode5, false, true);
+        txnIds5.add(txnId5);
+
+        return txnIds;
+    }
+
     private boolean containsTransaction(List<Transaction> txns, String txnId)
     {
     	boolean found = false;
@@ -296,6 +326,39 @@ public class TransactionCleanupTest
         assertNull("Node 5 was not cleaned up", nodeDAO.getNodeRefStatus(nodeRef5));
     }
 
+    @Test public void testPurgeNodeUseTransactionCommitTime() throws Exception
+    {
+        // make sure we clean up all the other nodes that may require purging
+        worker.setPurgeSize(7200000);// 2 hours
+        worker.doClean();
+        // delete the node 4 and node 5 with a half a second delay between the events
+        createTransactionsForNodePurgeTest();
+
+        // Double-check that n4 and n5 are present in deleted form
+        nodesCache.clear();
+
+        assertNotNull("Node 4 is deleted but not purged", nodeDAO.getNodeRefStatus(nodeRef4));
+        assertNotNull("Node 5 is deleted but not purged", nodeDAO.getNodeRefStatus(nodeRef5));
+
+        // run the transaction cleaner
+        worker.setPurgeSize(5); // small purge size
+        // we want to clean all the transactions starting with the fromCustomCommitTime
+        worker.setFromCustomCommitTime(fromCustomCommitTime);
+        List<String> reports = worker.doClean();
+        for (String report : reports)
+        {
+            logger.debug(report);
+        }
+
+        // only node 5 should be purged,
+        // node 4 should still be present as the transaction happened before fromCustomCommitTime
+        nodesCache.clear();
+
+        assertNotNull("Node 4 is deleted but not purged", nodeDAO.getNodeRefStatus(nodeRef4));
+
+        assertNull("Node 5 was not cleaned up", nodeDAO.getNodeRefStatus(nodeRef5));
+    }
+
     @After
     public void after()
     {