You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by tv...@apache.org on 2016/02/06 19:38:31 UTC

svn commit: r1728866 - in /commons/proper/jcs/trunk: commons-jcs-core/src/main/java/org/apache/commons/jcs/auxiliary/disk/block/ commons-jcs-core/src/test/java/org/apache/commons/jcs/auxiliary/disk/block/ src/changes/

Author: tv
Date: Sat Feb  6 18:38:31 2016
New Revision: 1728866

URL: http://svn.apache.org/viewvc?rev=1728866&view=rev
Log:
Add verification of block disk cache key file.

Modified:
    commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/auxiliary/disk/block/BlockDiskKeyStore.java
    commons/proper/jcs/trunk/commons-jcs-core/src/test/java/org/apache/commons/jcs/auxiliary/disk/block/BlockDiskCacheKeyStoreUnitTest.java
    commons/proper/jcs/trunk/src/changes/changes.xml

Modified: commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/auxiliary/disk/block/BlockDiskKeyStore.java
URL: http://svn.apache.org/viewvc/commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/auxiliary/disk/block/BlockDiskKeyStore.java?rev=1728866&r1=1728865&r2=1728866&view=diff
==============================================================================
--- commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/auxiliary/disk/block/BlockDiskKeyStore.java (original)
+++ commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/auxiliary/disk/block/BlockDiskKeyStore.java Sat Feb  6 18:38:31 2016
@@ -29,8 +29,11 @@ import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
+import java.util.TreeMap;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import org.apache.commons.jcs.auxiliary.disk.behavior.IDiskCacheAttributes.DiskLimitType;
@@ -70,7 +73,10 @@ public class BlockDiskKeyStore<K>
     /** The maximum number of keys to store in memory */
     private final int maxKeySize;
 
-    /** we need this so we can communicate free blocks to the data store when keys fall off the LRU */
+    /**
+     * we need this so we can communicate free blocks to the data store when
+     * keys fall off the LRU
+     */
     protected final BlockDiskCache<K, ?> blockDiskCache;
 
     private DiskLimitType diskLimitType = DiskLimitType.COUNT;
@@ -112,7 +118,12 @@ public class BlockDiskKeyStore<K>
         if (keyFile.length() > 0)
         {
             loadKeys();
-            // TODO verify somehow
+            if (!verify())
+            {
+                log.warn(logCacheName + "Key File is invalid. Resetting file.");
+                initKeyMap();
+                reset();
+            }
         }
         else
         {
@@ -121,8 +132,8 @@ public class BlockDiskKeyStore<K>
     }
 
     /**
-     * Saves key file to disk. This gets the LRUMap entry set and write the entries out one by one
-     * after putting them in a wrapper.
+     * Saves key file to disk. This gets the LRUMap entry set and write the
+     * entries out one by one after putting them in a wrapper.
      */
     protected void saveKeys()
     {
@@ -142,7 +153,12 @@ public class BlockDiskKeyStore<K>
                 ObjectOutputStream oos = new ObjectOutputStream(bos);
                 try
                 {
-                    // don't need to synchronize, since the underlying collection makes a copy
+                    if (!verify())
+                    {
+                        throw new IOException("Inconsistent key file");
+                    }
+                    // don't need to synchronize, since the underlying
+                    // collection makes a copy
                     for (Map.Entry<K, int[]> entry : keyHash.entrySet())
                     {
                         BlockDiskElementDescriptor<K> descriptor = new BlockDiskElementDescriptor<K>();
@@ -162,7 +178,7 @@ public class BlockDiskKeyStore<K>
             if (log.isInfoEnabled())
             {
                 log.info(logCacheName + "Finished saving keys. It took " + timer.getElapsedTimeString() + " to store " + numKeys
-                    + " keys.  Key file length [" + keyFile.length() + "]");
+                        + " keys.  Key file length [" + keyFile.length() + "]");
             }
         }
         catch (IOException e)
@@ -184,7 +200,8 @@ public class BlockDiskKeyStore<K>
     }
 
     /**
-     * This is mainly used for testing. It leave the disk in tact, and just clears memory.
+     * This is mainly used for testing. It leave the disk in tact, and just
+     * clears memory.
      */
     protected void clearMemoryMap()
     {
@@ -214,7 +231,8 @@ public class BlockDiskKeyStore<K>
         }
         else
         {
-            // If no max size, use a plain map for memory and processing efficiency.
+            // If no max size, use a plain map for memory and processing
+            // efficiency.
             keyHash = new HashMap<K, int[]>();
             // keyHash = Collections.synchronizedMap( new HashMap() );
             if (log.isInfoEnabled())
@@ -225,8 +243,8 @@ public class BlockDiskKeyStore<K>
     }
 
     /**
-     * Loads the keys from the .key file. The keys are stored individually on disk. They are added
-     * one by one to an LRUMap..
+     * Loads the keys from the .key file. The keys are stored individually on
+     * disk. They are added one by one to an LRUMap..
      */
     protected void loadKeys()
     {
@@ -282,7 +300,7 @@ public class BlockDiskKeyStore<K>
                 if (log.isInfoEnabled())
                 {
                     log.info(logCacheName + "Loaded keys from [" + fileName + "], key count: " + keyHash.size() + "; up to "
-                        + maxKeySize + " will be available.");
+                            + maxKeySize + " will be available.");
                 }
             }
         }
@@ -362,8 +380,53 @@ public class BlockDiskKeyStore<K>
     }
 
     /**
-     * Class for recycling and lru. This implements the LRU size overflow callback, so we can mark the
-     * blocks as free.
+     * Verify key store integrity
+     *
+     * @return true if key store is valid
+     */
+    private boolean verify()
+    {
+        Map<Integer, Set<K>> blockAllocationMap = new TreeMap<Integer, Set<K>>();
+        for (Entry<K, int[]> e : keyHash.entrySet())
+        {
+            for (int block : e.getValue())
+            {
+                Set<K> keys = blockAllocationMap.get(block);
+                if (keys == null)
+                {
+                    keys = new HashSet<K>();
+                    blockAllocationMap.put(block, keys);
+                }
+                else if (!log.isDebugEnabled())
+                {
+                    // keys are not null, and no debug - fail fast
+                    return false;
+                }
+                keys.add(e.getKey());
+            }
+        }
+        boolean ok = true;
+        if (log.isDebugEnabled())
+        {
+            for (Entry<Integer, Set<K>> e : blockAllocationMap.entrySet())
+            {
+                log.debug("Block " + e.getKey() + ":" + e.getValue());
+                if (e.getValue().size() > 1)
+                {
+                    ok = false;
+                }
+            }
+            return ok;
+        }
+        else
+        {
+            return ok;
+        }
+    }
+
+    /**
+     * Class for recycling and lru. This implements the LRU size overflow
+     * callback, so we can mark the blocks as free.
      */
     public class LRUMapSizeLimited extends AbstractLRUMap<K, int[]>
     {
@@ -385,7 +448,8 @@ public class BlockDiskKeyStore<K>
         }
 
         /**
-         * @param maxSize maximum cache size in kB
+         * @param maxSize
+         *            maximum cache size in kB
          */
         public LRUMapSizeLimited(int maxSize)
         {
@@ -450,8 +514,9 @@ public class BlockDiskKeyStore<K>
         }
 
         /**
-         * This is called when the may key size is reached. The least recently used item will be
-         * passed here. We will store the position and size of the spot on disk in the recycle bin.
+         * This is called when the may key size is reached. The least recently
+         * used item will be passed here. We will store the position and size of
+         * the spot on disk in the recycle bin.
          * <p>
          *
          * @param key
@@ -481,8 +546,8 @@ public class BlockDiskKeyStore<K>
     }
 
     /**
-     * Class for recycling and lru. This implements the LRU overflow callback, so we can mark the
-     * blocks as free.
+     * Class for recycling and lru. This implements the LRU overflow callback,
+     * so we can mark the blocks as free.
      */
     public class LRUMapCountLimited extends LRUMap<K, int[]>
     {
@@ -497,8 +562,9 @@ public class BlockDiskKeyStore<K>
         }
 
         /**
-         * This is called when the may key size is reached. The least recently used item will be
-         * passed here. We will store the position and size of the spot on disk in the recycle bin.
+         * This is called when the may key size is reached. The least recently
+         * used item will be passed here. We will store the position and size of
+         * the spot on disk in the recycle bin.
          * <p>
          *
          * @param key

Modified: commons/proper/jcs/trunk/commons-jcs-core/src/test/java/org/apache/commons/jcs/auxiliary/disk/block/BlockDiskCacheKeyStoreUnitTest.java
URL: http://svn.apache.org/viewvc/commons/proper/jcs/trunk/commons-jcs-core/src/test/java/org/apache/commons/jcs/auxiliary/disk/block/BlockDiskCacheKeyStoreUnitTest.java?rev=1728866&r1=1728865&r2=1728866&view=diff
==============================================================================
--- commons/proper/jcs/trunk/commons-jcs-core/src/test/java/org/apache/commons/jcs/auxiliary/disk/block/BlockDiskCacheKeyStoreUnitTest.java (original)
+++ commons/proper/jcs/trunk/commons-jcs-core/src/test/java/org/apache/commons/jcs/auxiliary/disk/block/BlockDiskCacheKeyStoreUnitTest.java Sat Feb  6 18:38:31 2016
@@ -19,156 +19,168 @@ package org.apache.commons.jcs.auxiliary
  * under the License.
  */
 
-import org.apache.commons.jcs.auxiliary.disk.behavior.IDiskCacheAttributes.DiskLimitType;
-
 import junit.framework.TestCase;
 
+import org.apache.commons.jcs.auxiliary.disk.behavior.IDiskCacheAttributes.DiskLimitType;
+
 /**
  * Tests for the keyStore.
  * <p>
+ *
  * @author Aaron Smuts
  */
 public class BlockDiskCacheKeyStoreUnitTest
-    extends TestCase
+        extends TestCase
 {
     /** Directory name */
     private final String rootDirName = "target/test-sandbox/block";
 
     /**
-     * Put a bunch of keys inthe key store and verify that they are present.
+     * Put a bunch of keys in the key store and verify that they are present.
      * <p>
+     *
      * @throws Exception
      */
     public void testPutKeys()
-        throws Exception
+            throws Exception
     {
         // SETUP
         BlockDiskCacheAttributes attributes = new BlockDiskCacheAttributes();
-        attributes.setCacheName( "testPutKeys" );
-        attributes.setDiskPath( rootDirName );
-        attributes.setMaxKeySize( 1000 );
-        attributes.setBlockSizeBytes( 2000 );
+        attributes.setCacheName("testPutKeys");
+        attributes.setDiskPath(rootDirName);
+        attributes.setMaxKeySize(1000);
+        attributes.setBlockSizeBytes(2000);
 
         innerTestPutKeys(attributes);
-            }
+    }
 
     public void testPutKeysSize()
             throws Exception
-            {
+    {
         // SETUP
         BlockDiskCacheAttributes attributes = new BlockDiskCacheAttributes();
-        attributes.setCacheName( "testPutKeys" );
-        attributes.setDiskPath( rootDirName );
-        attributes.setMaxKeySize( 100000 );
-        attributes.setBlockSizeBytes( 1024 );
+        attributes.setCacheName("testPutKeys");
+        attributes.setDiskPath(rootDirName);
+        attributes.setMaxKeySize(100000);
+        attributes.setBlockSizeBytes(1024);
         attributes.setDiskLimitType(DiskLimitType.SIZE);
 
         innerTestPutKeys(attributes);
-            }
-
-
-    private void innerTestPutKeys(BlockDiskCacheAttributes attributes) {
-        BlockDiskCache<String, String> blockDiskCache = new BlockDiskCache<String, String>( attributes );
+    }
 
-        BlockDiskKeyStore<String> keyStore = new BlockDiskKeyStore<String>( attributes, blockDiskCache );
+    private void innerTestPutKeys(BlockDiskCacheAttributes attributes)
+    {
+        BlockDiskCache<String, String> blockDiskCache = new BlockDiskCache<String, String>(attributes);
+        BlockDiskKeyStore<String> keyStore = new BlockDiskKeyStore<String>(attributes, blockDiskCache);
 
         // DO WORK
         int numElements = 100;
-        for ( int i = 0; i < numElements; i++ )
+        for (int i = 0; i < numElements; i++)
         {
-            keyStore.put( String.valueOf( i ), new int[i] );
+            keyStore.put(String.valueOf(i), new int[i]);
         }
-//        System.out.println( "testPutKeys " + keyStore );
+        // System.out.println( "testPutKeys " + keyStore );
 
         // VERIFY
-        assertEquals( "Wrong number of keys", numElements, keyStore.size() );
-        for ( int i = 0; i < numElements; i++ )
+        assertEquals("Wrong number of keys", numElements, keyStore.size());
+        for (int i = 0; i < numElements; i++)
         {
-            int[] result = keyStore.get( String.valueOf( i ) );
-            assertEquals( "Wrong array returned.", i, result.length );
+            int[] result = keyStore.get(String.valueOf(i));
+            assertEquals("Wrong array returned.", i, result.length);
         }
     }
 
     /**
-     * Verify that we can load keys that we saved. Add a bunch. Save them. Clear the memory keyhash.
-     * Load the keys. Verify.
+     * Verify that we can load keys that we saved. Add a bunch. Save them. Clear
+     * the memory key hash. Load the keys. Verify.
      * <p>
+     *
      * @throws Exception
      */
     public void testSaveLoadKeys()
-        throws Exception
+            throws Exception
     {
         // SETUP
         BlockDiskCacheAttributes attributes = new BlockDiskCacheAttributes();
-        attributes.setCacheName( "testSaveLoadKeys" );
-        attributes.setDiskPath( rootDirName );
-        attributes.setMaxKeySize( 10000 );
-        attributes.setBlockSizeBytes( 2000 );
+        attributes.setCacheName("testSaveLoadKeys");
+        attributes.setDiskPath(rootDirName);
+        attributes.setMaxKeySize(10000);
+        attributes.setBlockSizeBytes(2000);
 
         testSaveLoadKeysInner(attributes);
-            }
+    }
 
     public void testSaveLoadKeysSize()
             throws Exception
-            {
+    {
         // SETUP
         BlockDiskCacheAttributes attributes = new BlockDiskCacheAttributes();
-        attributes.setCacheName( "testSaveLoadKeys" );
-        attributes.setDiskPath( rootDirName );
-        attributes.setMaxKeySize( 10000 );
-        attributes.setBlockSizeBytes( 2000 );
+        attributes.setCacheName("testSaveLoadKeys");
+        attributes.setDiskPath(rootDirName);
+        attributes.setMaxKeySize(10000);
+        attributes.setBlockSizeBytes(2000);
 
         testSaveLoadKeysInner(attributes);
-            }
-
-
+    }
 
-    private void testSaveLoadKeysInner(BlockDiskCacheAttributes attributes) {
-        BlockDiskKeyStore<String> keyStore = new BlockDiskKeyStore<String>( attributes, null );
+    private void testSaveLoadKeysInner(BlockDiskCacheAttributes attributes)
+    {
+        BlockDiskKeyStore<String> keyStore = new BlockDiskKeyStore<String>(attributes, null);
 
         // DO WORK
         int numElements = 1000;
-        //Random random = new Random( 89 );
-        for ( int i = 0; i < numElements; i++ )
+        int blockIndex = 0;
+        // Random random = new Random( 89 );
+        for (int i = 0; i < numElements; i++)
         {
-            int blocks = i;//random.nextInt( 10 );
-            keyStore.put( String.valueOf( i ), new int[blocks] );
-            keyStore.put( String.valueOf( i ), new int[i] );
+            int blocks = i;// random.nextInt( 10 );
+
+            // fill with reasonable data to make verify() happy
+            int[] block1 = new int[blocks];
+            int[] block2 = new int[blocks];
+            for (int j = 0; j < blocks; j++)
+            {
+                block1[j] = blockIndex++;
+                block2[j] = blockIndex++;
+            }
+            keyStore.put(String.valueOf(i), block1);
+            keyStore.put(String.valueOf(i), block2);
         }
-//        System.out.println( "testSaveLoadKeys " + keyStore );
+        // System.out.println( "testSaveLoadKeys " + keyStore );
 
         // VERIFY
-        assertEquals( "Wrong number of keys", numElements, keyStore.size() );
+        assertEquals("Wrong number of keys", numElements, keyStore.size());
 
         // DO WORK
         keyStore.saveKeys();
         keyStore.clearMemoryMap();
 
         // VERIFY
-        assertEquals( "Wrong number of keys after clearing memory", 0, keyStore.size() );
+        assertEquals("Wrong number of keys after clearing memory", 0, keyStore.size());
 
         // DO WORK
         keyStore.loadKeys();
 
         // VERIFY
-        assertEquals( "Wrong number of keys after loading", numElements, keyStore.size() );
-        for ( int i = 0; i < numElements; i++ )
+        assertEquals("Wrong number of keys after loading", numElements, keyStore.size());
+        for (int i = 0; i < numElements; i++)
         {
-            int[] result = keyStore.get( String.valueOf( i ) );
-            assertEquals( "Wrong array returned.", i, result.length );
+            int[] result = keyStore.get(String.valueOf(i));
+            assertEquals("Wrong array returned.", i, result.length);
         }
     }
 
-    public void testObjectLargerThanMaxSize() {
+    public void testObjectLargerThanMaxSize()
+    {
         BlockDiskCacheAttributes attributes = new BlockDiskCacheAttributes();
-        attributes.setCacheName( "testObjectLargerThanMaxSize" );
-        attributes.setDiskPath( rootDirName );
-        attributes.setMaxKeySize( 1000 );
-        attributes.setBlockSizeBytes( 2000 );
+        attributes.setCacheName("testObjectLargerThanMaxSize");
+        attributes.setDiskPath(rootDirName);
+        attributes.setMaxKeySize(1000);
+        attributes.setBlockSizeBytes(2000);
         attributes.setDiskLimitType(DiskLimitType.SIZE);
 
         @SuppressWarnings({ "unchecked", "rawtypes" })
-        BlockDiskKeyStore<String> keyStore = new BlockDiskKeyStore<String>( attributes, new BlockDiskCache(attributes));
+        BlockDiskKeyStore<String> keyStore = new BlockDiskKeyStore<String>(attributes, new BlockDiskCache(attributes));
 
         keyStore.put("1", new int[1000]);
         keyStore.put("2", new int[1000]);

Modified: commons/proper/jcs/trunk/src/changes/changes.xml
URL: http://svn.apache.org/viewvc/commons/proper/jcs/trunk/src/changes/changes.xml?rev=1728866&r1=1728865&r2=1728866&view=diff
==============================================================================
--- commons/proper/jcs/trunk/src/changes/changes.xml (original)
+++ commons/proper/jcs/trunk/src/changes/changes.xml Sat Feb  6 18:38:31 2016
@@ -20,6 +20,9 @@
 	</properties>
 	<body>
         <release version="2.0" date="unreleased" description="JDK 1.6 based major release">
+            <action dev="tv" type="add" due-to="Wiktor Niesiobedzki">
+                Add verification of block disk cache key file.
+            </action>
             <action issue="JCS-159" dev="tv" type="fix" due-to="Wiktor Niesiobedzki">
                 Fix: BlockDiskCache overwrites data after loading from disk
             </action>