Merged BRANCHES/DEV/THOR1 to HEAD:

32573: THOR-659: Caching Content Store blocked readers


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@32628 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

source/java/org/alfresco/repo/content/caching/CachingContentStore.java

@@ -54,7 +54,7 @@ public class CachingContentStore implements ContentStore, ApplicationEventPublis
 {
     private final static Log log = LogFactory.getLog(CachingContentStore.class);
     // NUM_LOCKS absolutely must be a power of 2 for the use of locks to be evenly balanced
-    private final static int numLocks = 32;
+    private final static int numLocks = 256;
     private final static ReentrantReadWriteLock[] locks; 
     private ContentStore backingStore;
     private ContentCache cache;

source/java/org/alfresco/repo/content/caching/cleanup/CachedContentCleaner.java

@@ -40,7 +40,7 @@ import org.springframework.context.ApplicationEventPublisherAware;
  * 
  * @author Matt Ward
  */
-public class CachedContentCleaner implements FileHandler, ApplicationEventPublisherAware
+public class CachedContentCleaner extends Thread implements FileHandler, ApplicationEventPublisherAware
 {
     private static final Log log = LogFactory.getLog(CachedContentCleaner.class);
     private ContentCacheImpl cache;   // impl specific functionality required
@@ -58,85 +58,102 @@ public class CachedContentCleaner implements FileHandler, ApplicationEventPublis
     private Date timeFinished;
     private ApplicationEventPublisher eventPublisher;
     private long targetReductionBytes;
+    private boolean cleanRequested;
+    private String reasonMessage;
+   
+    
+    public CachedContentCleaner()
+    {        
+        setName(getClass().getSimpleName());
+        setDaemon(true);
+    }
     
     /**
-     * This method should be called after the cleaner has been fully constructed
+     * This method MUST be called after the cleaner has been fully constructed
-     * to notify interested parties that the cleaner exists.
+     * to notify interested parties that the cleaner exists and to start the actual cleaner thread.
      */
     public void init()
     {
         eventPublisher.publishEvent(new CachedContentCleanerCreatedEvent(this));    
+        start();
     }
     
-    public void execute()
+    
+    @Override
+    public void run()
+    {
+        while (true)
+        {
+            doClean();
+        }
+    }
+
+    public synchronized void execute()
     {
         execute("none specified");
     }
     
-    public void executeAggressive(String reason, long targetReductionBytes)
+    public synchronized void executeAggressive(String reason, long targetReductionBytes)
     {
         this.targetReductionBytes = targetReductionBytes;
         execute(reason);
-        this.targetReductionBytes = 0;
     }
     
-    public void execute(String reason)
+    private synchronized void doClean()
     {
-        lock.readLock().lock();
+        while (running || (!cleanRequested))
-        try
         {
-            if (running)
+            try
             {
-                // Do nothing - we only want one cleaner running at a time.
+                wait();
-                return;
+            }
+            catch (InterruptedException error)
+            {
+                // Nothing to do.
             }
         }
-        finally
-        {
-            lock.readLock().unlock();
-        }
-        lock.writeLock().lock();
-        try
-        {
-            if (!running)
-            {
-                if (log.isInfoEnabled())
-                {
-                    log.info("Starting cleaner, reason: " + reason);
-                }
-                running = true;
-                resetStats();
-                timeStarted = new Date();
-                cache.processFiles(this);
-                timeFinished = new Date(); 
         
-                if (usageTracker != null)
+        running = true;
-                {
+        if (log.isInfoEnabled())
-                    usageTracker.setCurrentUsageBytes(newDiskUsage);
-                }
-                
-                running = false;
-                if (log.isInfoEnabled())
-                {
-                    log.info("Finished, duration: " + getDurationSeconds() + "s, seen: " + numFilesSeen +
-                                ", marked: " + numFilesMarked +
-                                ", deleted: " + numFilesDeleted +
-                                " (" + String.format("%.2f", getSizeFilesDeletedMB()) + "MB, " +
-                                sizeFilesDeleted + " bytes)" +
-                                ", target: " + targetReductionBytes + " bytes");
-                }
-            }
-        }
-        finally
         {
-            lock.writeLock().unlock();
+            log.info("Starting cleaner, reason: " + reasonMessage);
         }
+        resetStats();
+        timeStarted = new Date();
+        cache.processFiles(this);
+        timeFinished = new Date(); 
+        
+        if (usageTracker != null)
+        {
+            usageTracker.setCurrentUsageBytes(newDiskUsage);
+        }
+        
+        if (log.isInfoEnabled())
+        {
+            log.info("Finished, duration: " + getDurationSeconds() + "s, seen: " + numFilesSeen +
+                        ", marked: " + numFilesMarked +
+                        ", deleted: " + numFilesDeleted +
+                        " (" + String.format("%.2f", getSizeFilesDeletedMB()) + "MB, " +
+                        sizeFilesDeleted + " bytes)" +
+                        ", target: " + targetReductionBytes + " bytes");
+        }
+        
+        cleanRequested = false;
+        this.targetReductionBytes = 0;
+        running = false;
+        
+        notifyAll();
+    }
+    
+    
+    public synchronized void execute(String reasonMessage)
+    {
+        this.reasonMessage = reasonMessage;
+        cleanRequested = true;
+        notifyAll();
     }
     
     
-    /**
-     * 
-     */
     private void resetStats()
     {
         newDiskUsage = 0;
@@ -152,7 +169,7 @@ public class CachedContentCleaner implements FileHandler, ApplicationEventPublis
     {
         if (log.isDebugEnabled())
         {
-            log.debug("handle file: " + cachedContentFile);
+            log.debug("handle file: " + cachedContentFile + " (target reduction: " + targetReductionBytes + " bytes)");
         }
         numFilesSeen++;
         CacheFileProps props = null;
@@ -160,6 +177,11 @@ public class CachedContentCleaner implements FileHandler, ApplicationEventPublis
         
         if (targetReductionBytes > 0 && sizeFilesDeleted < targetReductionBytes)
         {
+            if (log.isDebugEnabled())
+            {
+                log.debug("Target reduction " + targetReductionBytes +
+                           " bytes not yet reached. Deleted so far: " + sizeFilesDeleted);
+            }
             // Aggressive clean mode, delete file straight away.
             deleted = deleteFilesNow(cachedContentFile);
         }
@@ -299,10 +321,21 @@ public class CachedContentCleaner implements FileHandler, ApplicationEventPublis
         boolean deleted = cacheFile.delete();
         if (deleted)
         {
+            if (log.isTraceEnabled())
+            {
+                log.trace("Deleted cache file: " + cacheFile);
+            }
             numFilesDeleted++;
             sizeFilesDeleted += fileSize;
             Deleter.deleteEmptyParents(cacheFile, cache.getCacheRoot());
         }
+        else
+        {
+            if (log.isWarnEnabled())
+            {
+                log.warn("Failed to delete cache file: " + cacheFile);
+            }
+        }
         
         return deleted;
     }

source/java/org/alfresco/repo/content/caching/cleanup/CachedContentCleanupJobTest.java

@@ -87,7 +87,7 @@ public class CachedContentCleanupJobTest
 
     
     @Test
-    public void filesNotInCacheAreDeleted()
+    public void filesNotInCacheAreDeleted() throws InterruptedException
     {
         cleaner.setMaxDeleteWatchCount(0);
         int numFiles = 300; // Must be a multiple of number of UrlSource types being tested
@@ -108,6 +108,12 @@ public class CachedContentCleanupJobTest
         // Run cleaner
         cleaner.execute();
         
+        Thread.sleep(400);
+        while (cleaner.isRunning())
+        {
+            Thread.sleep(200);
+        }
+        
         // check all files deleted
         for (File file : files)
         {
@@ -149,6 +155,12 @@ public class CachedContentCleanupJobTest
         // old the test will fail and it will be necessary to rethink how to test this.
         cleaner.execute();
         
+        Thread.sleep(400);
+        while (cleaner.isRunning())
+        {
+            Thread.sleep(200);
+        }  
+        
         // check all 'old' files deleted
         for (File file : oldFiles)
         {
@@ -186,6 +198,12 @@ public class CachedContentCleanupJobTest
         // Since some of the newer files are not in the cache, it will delete those.
         cleaner.executeAggressive("aggressiveCleanReclaimsTargetSpace()", sevenFilesSize);
         
+        Thread.sleep(400);
+        while (cleaner.isRunning())
+        {
+            Thread.sleep(200);
+        }
+        
         int numDeleted = 0;
         
         for (File f : files)
@@ -239,6 +257,12 @@ public class CachedContentCleanupJobTest
         // Since some of the newer files are not in the cache, it will delete those too.
         cleaner.executeAggressive("standardCleanAfterAggressiveFinished()", sevenFilesSize);
                 
+        Thread.sleep(400);
+        while (cleaner.isRunning())
+        {
+            Thread.sleep(200);
+        }
+        
         for (int i = 0; i < numFiles; i++)
         {
             if (i < 7)
@@ -328,7 +352,7 @@ public class CachedContentCleanupJobTest
     
     
     @Test
-    public void filesInCacheAreNotDeleted()
+    public void filesInCacheAreNotDeleted() throws InterruptedException
     {
         cleaner.setMaxDeleteWatchCount(0);
         
@@ -344,6 +368,12 @@ public class CachedContentCleanupJobTest
         
         cleaner.execute();
         
+        Thread.sleep(400);
+        while (cleaner.isRunning())
+        {
+            Thread.sleep(200);
+        }
+        
         for (int i = 0; i < numFiles; i++)
         {
             File cacheFile = new File(cache.getCacheFilePath(url));

source/java/org/alfresco/repo/content/caching/quota/StandardQuotaStrategy.java

@@ -88,7 +88,7 @@ public class StandardQuotaStrategy implements QuotaManagerStrategy, UsageTracker
         
         loadDiskUsage();
         // Run the cleaner thread so that it can update the disk usage more accurately.
-        runCleanerThread("quota (init)");
+        signalCleanerStart("quota (init)");
     }
     
     
@@ -170,7 +170,7 @@ public class StandardQuotaStrategy implements QuotaManagerStrategy, UsageTracker
                 log.debug("Panic threshold reached (" + panicThresholdPct +
                             "%) - vetoing disk write and starting cached content cleaner.");
             }
-            runCleanerThread("quota (panic threshold)");
+            signalCleanerStart("quota (panic threshold)");
             return false;
         }
         
@@ -203,7 +203,7 @@ public class StandardQuotaStrategy implements QuotaManagerStrategy, UsageTracker
                 log.debug("Usage has reached or exceeded quota limit, limit: " + maxUsageBytes +
                             " bytes, current usage: " + getCurrentUsageBytes() + " bytes.");
             }
-            runAggressiveCleanerThread("quota (limit reached)");
+            signalAggressiveCleanerStart("quota (limit reached)");
         }
         else if (usageHasReached(cleanThresholdPct))
         {
@@ -213,7 +213,7 @@ public class StandardQuotaStrategy implements QuotaManagerStrategy, UsageTracker
                 log.debug("Usage has reached " + cleanThresholdPct + "% - starting cached content cleaner.");
             }
             
-            runCleanerThread("quota (clean threshold)");
+            signalCleanerStart("quota (clean threshold)");
         }
         
         return keepNewFile;
@@ -223,27 +223,17 @@ public class StandardQuotaStrategy implements QuotaManagerStrategy, UsageTracker
     /**
      * Run the cleaner in a new thread.
      */
-    private void runCleanerThread(final String reason, final boolean aggressive)
+    private void signalCleanerStart(final String reason, final boolean aggressive)
     {
-        Runnable cleanerRunner = new Runnable()
+        if (aggressive)
         {
-            @Override
+            long targetReductionBytes = (long) (((double) targetUsagePct / 100) * maxUsageBytes);
-            public void run()
+            cleaner.executeAggressive(reason, targetReductionBytes);                    
-            {
+        }
-                if (aggressive)
+        else
-                {
+        {
-                    long targetReductionBytes = (long) (((double) targetUsagePct / 100) * maxUsageBytes);
+            cleaner.execute(reason);
-                    cleaner.executeAggressive(reason, targetReductionBytes);                    
+        }
-                }
-                else
-                {
-                    cleaner.execute(reason);
-                }
-            }
-        };
-        Thread cleanerThread = new Thread(cleanerRunner, getClass().getSimpleName() + " cleaner");
-        cleanerThread.setDaemon(true);
-        cleanerThread.start();
     }
     
     /**
@@ -251,9 +241,9 @@ public class StandardQuotaStrategy implements QuotaManagerStrategy, UsageTracker
      * 
      * @param reason
      */
-    private void runCleanerThread(final String reason)
+    private void signalCleanerStart(final String reason)
     {
-        runCleanerThread(reason, false);
+        signalCleanerStart(reason, false);
     }
     
     /**
@@ -261,9 +251,9 @@ public class StandardQuotaStrategy implements QuotaManagerStrategy, UsageTracker
      * 
      * @param reason
      */
-    private void runAggressiveCleanerThread(final String reason)
+    private void signalAggressiveCleanerStart(final String reason)
     {
-       runCleanerThread(reason, true); 
+       signalCleanerStart(reason, true); 
     }
 
     

source/test-resources/cachingstore/test-cleaner-context.xml

@@ -9,7 +9,9 @@
         <property name="pauseMillis" value="0"/>
     </bean>
     
-    <bean id="cachedContentCleaner" class="org.alfresco.repo.content.caching.cleanup.CachedContentCleaner">
+    <bean id="cachedContentCleaner"
+        class="org.alfresco.repo.content.caching.cleanup.CachedContentCleaner"
+        init-method="init">
         <property name="maxDeleteWatchCount" value="1"/>
         <property name="cache" ref="contentCache"/>
     </bean>

source/test-resources/cachingstore/test-std-quota-context.xml

@@ -56,7 +56,9 @@
     </bean>
     
     
-    <bean id="cachedContentCleaner" class="org.alfresco.repo.content.caching.cleanup.CachedContentCleaner">
+    <bean id="cachedContentCleaner"
+        class="org.alfresco.repo.content.caching.cleanup.CachedContentCleaner"
+        init-method="init">
         <property name="maxDeleteWatchCount" value="0"/><!-- zero is NOT recommmend in production -->
         <property name="cache" ref="contentCache"/>
         <property name="usageTracker" ref="quotaManager"/>