Merged HEAD-BUG-FIX (5.1/Cloud) to HEAD (5.1/Cloud)

93938: Merged BRANCHES/DEV/mward/post50_hbf_fixes to BRANCHES/DEV/HEAD-BUG-FIX:
      93561: Cache stats: Fixed concurrency issue with 'get' count. Added method to retrieve stats snapshot.


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

source/java/org/alfresco/repo/cache/CacheStatistics.java

@@ -18,6 +18,8 @@
  */
 package org.alfresco.repo.cache;
 
+import java.util.Map;
+
 import org.alfresco.repo.cache.TransactionStats.OpType;
 
 /**
@@ -63,5 +65,25 @@ public interface CacheStatistics
      * @param cacheName  The cache name.
      * @return ratio (double)
      */
-    double hitMissRatio(String cacheName); 
+    double hitMissRatio(String cacheName);
+    
+    /**
+     * Retrieve the total number of get operations invoked on the
+     * cache (i.e. sum of hits and misses).
+     * 
+     * @param cacheName  The cache name. 
+     * @return Count of get operations.
+     */
+    long numGets(String cacheName);
+    
+    /**
+     * Retrieve a map containing a snapshot of all of the raw stats
+     * (e.g. counts, mean operation times etc.). Since this is a snapshot
+     * it is unaffected by future modifications to the statistics - by
+     * using {@link CacheStatistics#add(String, TransactionStats)} for example.
+     * 
+     * @param cacheName  The cache name.
+     * @return Map of OpType to OperationStats
+     */
+    Map<OpType, OperationStats> allStats(String cacheName);
 }

source/java/org/alfresco/repo/cache/InMemoryCacheStatistics.java

@@ -60,7 +60,7 @@ public class InMemoryCacheStatistics implements CacheStatistics, ApplicationCont
                 throw new NoStatsForCache(cacheName);
             }
             OperationStats opStats = cacheStats.get(opType);
-            return opStats.count;
+            return opStats.getCount();
         }
         finally
         {
@@ -150,8 +150,8 @@ public class InMemoryCacheStatistics implements CacheStatistics, ApplicationCont
             {
                 throw new NoStatsForCache(cacheName);
             }
-            long hits = cacheStats.get(OpType.GET_HIT).count;
+            long hits = cacheStats.get(OpType.GET_HIT).getCount();
-            long misses = cacheStats.get(OpType.GET_MISS).count;
+            long misses = cacheStats.get(OpType.GET_MISS).getCount();
             return (double)hits / (hits+misses);
         }
         finally
@@ -159,40 +159,46 @@ public class InMemoryCacheStatistics implements CacheStatistics, ApplicationCont
             readLock.unlock();
         }
     }
-
+    
-    /**
+    @Override
-     * Represents a single cache operation type's statistics.
+    public long numGets(String cacheName)
-     */
-    private static final class OperationStats
     {
-        /** Total time spent in operations of this type */
+        ReadLock readLock = getReadLock(cacheName);
-        private final double totalTime;
+        readLock.lock();
-        /** Count of how many instances of this operation occurred. */
+        try
-        private final long count;
-        
-        public OperationStats(double totalTime, long count)
         {
-            this.totalTime = totalTime;
+            Map<OpType, OperationStats> cacheStats = cacheToStatsMap.get(cacheName);
-            this.count = count;
+            if (cacheStats == null)
-        }
-        
-        public OperationStats(OperationStats source, double totalTime, long count)
-        {
-            if (Double.compare(source.totalTime, Double.NaN) == 0)
             {
-                // No previous time to add new time to.
+                throw new NoStatsForCache(cacheName);
-                this.totalTime = totalTime;
             }
-            else
+            long hits = cacheStats.get(OpType.GET_HIT).getCount();
-            {
+            long misses = cacheStats.get(OpType.GET_MISS).getCount();
-                this.totalTime = source.totalTime + totalTime;
+            return hits+misses;
-            }
-            this.count = source.count + count;
         }
-        
+        finally
-        public double meanTime()
         {
-            return totalTime / count;
+            readLock.unlock();
+        }
+    }
+    
+    @Override
+    public Map<OpType, OperationStats> allStats(String cacheName)
+    {
+        ReadLock readLock = getReadLock(cacheName);
+        readLock.lock();
+        try
+        {
+            Map<OpType, OperationStats> cacheStats = cacheToStatsMap.get(cacheName);
+            if (cacheStats == null)
+            {
+                throw new NoStatsForCache(cacheName);
+            }
+            return new HashMap<>(cacheStats);
+        }
+        finally
+        {
+            readLock.unlock();
         }
     }
 

source/java/org/alfresco/repo/cache/NoOpCacheStatistics.java

@@ -18,6 +18,9 @@
  */
 package org.alfresco.repo.cache;
 
+import java.util.HashMap;
+import java.util.Map;
+
 import org.alfresco.repo.cache.TransactionStats.OpType;
 
 /**
@@ -53,4 +56,16 @@ public class NoOpCacheStatistics implements CacheStatistics
     {
         return Double.NaN;
     }
+
+    @Override
+    public long numGets(String cacheName)
+    {
+        return 0;
+    }
+
+    @Override
+    public Map<OpType, OperationStats> allStats(String cacheName)
+    {
+        return new HashMap<>();
+    }
 }

source/java/org/alfresco/repo/cache/OperationStats.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright (C) 2005-2015 Alfresco Software Limited.
+ *
+ * This file is part of Alfresco
+ *
+ * Alfresco is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Alfresco is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.alfresco.repo.cache;
+
+
+/**
+ * Represents a single cache operation type's statistics.
+ * For example, the cummalative time spent performing a cache's remove operation
+ * and the number of times that the remove was performed.
+ * <p>
+ * Instances are immutable.
+ */
+public final class OperationStats
+{
+    /** Total time spent in operations of this type */
+    private final double totalTime;
+    /** Count of how many instances of this operation occurred. */
+    private final long count;
+    
+    public OperationStats(double totalTime, long count)
+    {
+        this.totalTime = totalTime;
+        this.count = count;
+    }
+    
+    public OperationStats(OperationStats source, double totalTime, long count)
+    {
+        if (Double.compare(source.totalTime, Double.NaN) == 0)
+        {
+            // No previous time to add new time to.
+            this.totalTime = totalTime;
+        }
+        else
+        {
+            this.totalTime = source.totalTime + totalTime;
+        }
+        this.count = source.count + count;
+    }
+    
+    public double meanTime()
+    {
+        return totalTime / count;
+    }
+
+    public double getTotalTime()
+    {
+        return this.totalTime;
+    }
+
+    public long getCount()
+    {
+        return this.count;
+    }
+}

source/test-java/org/alfresco/repo/cache/InMemoryCacheStatisticsTest.java

@@ -2,6 +2,8 @@ package org.alfresco.repo.cache;
 
 import static org.junit.Assert.*;
 
+import java.util.Map;
+
 import org.alfresco.repo.cache.TransactionStats.OpType;
 import org.junit.Before;
 import org.junit.Test;
@@ -60,6 +62,24 @@ public class InMemoryCacheStatisticsTest
         {
             // Good.
         }
+        try
+        {
+            cacheStats.numGets("cache1");
+            fail("NoStatsForCache should have been thrown.");
+        }
+        catch(NoStatsForCache e)
+        {
+            // Good.
+        }
+        try
+        {
+            cacheStats.allStats("cache1");
+            fail("NoStatsForCache should have been thrown.");
+        }
+        catch(NoStatsForCache e)
+        {
+            // Good.
+        }
     }
     
     @Test
@@ -141,5 +161,47 @@ public class InMemoryCacheStatisticsTest
         assertEquals(5, cacheStats.count("cache1", OpType.GET_HIT));
         assertEquals(1, cacheStats.count("cache1", OpType.GET_MISS));
         assertEquals(0.83, cacheStats.hitMissRatio("cache1"), 0.01d);
+        
+        // Check hit+miss count
+        assertEquals(6, cacheStats.numGets("cache1"));
+        
+        // Stats snapshot map
+        Map<OpType, OperationStats> snapshot = cacheStats.allStats("cache1");
+        assertEquals(5, snapshot.get(OpType.GET_HIT).getCount());
+        assertEquals(1, snapshot.get(OpType.GET_MISS).getCount());
+    }
+    
+    @Test
+    public void canRetrieveSnapshotOfAllStats()
+    {
+        TransactionStats txStats = new TransactionStats();
+        txStats.record(0, 1000, OpType.GET_HIT);
+        
+        // Add first statistical datapoint.
+        cacheStats.add("cache1", txStats);
+        
+        // Cache stats should be visible
+        Map<OpType, OperationStats> snapshot1 = cacheStats.allStats("cache1");
+        assertEquals(1, snapshot1.get(OpType.GET_HIT).getCount());
+        assertEquals(1000, snapshot1.get(OpType.GET_HIT).getTotalTime(), 0.0d);
+        // Map is fully populated
+        assertEquals(0, snapshot1.get(OpType.CLEAR).getCount());
+        assertEquals(0, snapshot1.get(OpType.PUT).getCount());
+        assertEquals(0, snapshot1.get(OpType.REMOVE).getCount());
+        assertEquals(0, snapshot1.get(OpType.GET_MISS).getCount());
+        
+        // Record further data
+        txStats = new TransactionStats();
+        txStats.record(0, 2000, OpType.GET_HIT);
+        cacheStats.add("cache1", txStats);
+        
+        // Check new snapshot reflects update
+        Map<OpType, OperationStats> snapshot2 = cacheStats.allStats("cache1");
+        assertEquals(2, snapshot2.get(OpType.GET_HIT).getCount());
+        assertEquals(3000, snapshot2.get(OpType.GET_HIT).getTotalTime(), 0.0d);
+        
+        // Check old snapshot is not affected
+        assertEquals(1, snapshot1.get(OpType.GET_HIT).getCount());
+        assertEquals(1000, snapshot1.get(OpType.GET_HIT).getTotalTime(), 0.0d);
     }
 }