o fixed an issue of missing pages after PageReclaimer runs by calling updateRecordMan...
authorKiran Ayyagari <kayyagari@apache.org>
Mon, 29 Jun 2015 07:52:05 +0000 (07:52 +0000)
committerKiran Ayyagari <kayyagari@apache.org>
Mon, 29 Jun 2015 07:52:05 +0000 (07:52 +0000)
o removed explicit transaction calls inside reclaim() method
o made some methods default protected in RM
o updated tests

mavibot/src/main/java/org/apache/directory/mavibot/btree/PageReclaimer.java
mavibot/src/main/java/org/apache/directory/mavibot/btree/RecordManager.java
mavibot/src/test/java/org/apache/directory/mavibot/btree/PageReclaimerTest.java

index a140c8a..c2bfe38 100644 (file)
@@ -21,6 +21,7 @@ package org.apache.directory.mavibot.btree;
 
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
@@ -97,9 +98,7 @@ public class PageReclaimer
                 // the revision last removed from copiedPage BTree
                 long lastRemovedRev = -1;
 
-                // FIXME an additional txn needs to be started to safeguard the copiedPage BTree changes
-                // no clue yet on why this is needed 
-                rm.beginTransaction();
+                List<Long> freeList = new ArrayList<Long>();
                 
                 for ( RevisionOffset ro : copiedRevisions )
                 {
@@ -114,32 +113,63 @@ public class PageReclaimer
 
                     //System.out.println( "Reclaiming " + Arrays.toString( offsets ) + "( " + offsets.length + " ) pages of the revision " + rv + " of BTree " + name );
 
-                    rm.free( offsets );
+                    for( long l : offsets )
+                    {
+                        freeList.add( l );
+                    }
 
                     RevisionName key = new RevisionName( rv, name );
                     
+                    //System.out.println( "delete cpb key " + key );
                     rm.copiedPageBtree.delete( key );
                     lastRemovedRev = rv;
                 }
 
-                rm.commit();
-                
                 // no new txn is needed for the operations on BoB
-                if ( lastRemovedRev != -1 )
+                // and also no need to traverse BoB if the tree is a sub-btree
+                if ( ( lastRemovedRev != -1 ) && !tree.isAllowDuplicates() )
                 {
                     // we SHOULD NOT delete the latest revision from BoB
                     NameRevision nr = new NameRevision( name, latestRev );
                     TupleCursor<NameRevision, Long> cursor = rm.btreeOfBtrees.browseFrom( nr );
                     
+                    List<Long> btreeHeaderOffsets = new ArrayList<Long>();
+                    
                     while ( cursor.hasPrev() )
                     {
                         Tuple<NameRevision, Long> t = cursor.prev();
                         //System.out.println( "deleting BoB rev " + t.getKey()  + " latest rev " + latestRev );
                         rm.btreeOfBtrees.delete( t.getKey() );
+                        btreeHeaderOffsets.add( t.value );
                     }
 
                     cursor.close();
+                    
+                    for( Long l : btreeHeaderOffsets )
+                    {
+                        // the offset may have already been present while
+                        // clearing CPB so skip it here, otherwise it will result in OOM
+                        // due to the attempt to free and already freed page
+                        if(freeList.contains( l ))
+                        {
+                            //System.out.println( "bob duplicate offset " + l );
+                            continue;
+                        }
+
+                        freeList.add( l );
+                    }
                 }
+                
+                for( Long offset : freeList )
+                {
+                    PageIO[] pageIos = rm.readPageIOs( offset, -1L );
+                    
+                    for ( PageIO pageIo : pageIos )
+                    {
+                        rm.free( pageIo );
+                    }
+                }
+
             }
 
             running = false;
index fab3e59..3b2fd1e 100644 (file)
@@ -218,7 +218,7 @@ public class RecordManager extends AbstractTransactionManager
     /** the threshold at which the PageReclaimer will be run to free the copied pages */
     // FIXME the below value is derived after seeing that anything higher than that
     // is resulting in a "This thread does not hold the transactionLock" error
-    private int spaceReclaimerThreshold = 70;
+    private int pageReclaimerThreshold = 70;
     
     /* a flag used to disable the free page reclaimer (used for internal testing only) */
     private boolean disableReclaimer = false;
@@ -320,6 +320,8 @@ public class RecordManager extends AbstractTransactionManager
         {
             commitCount = 0;
             reclaimer.reclaim();
+            // must update the headers after reclaim operation
+            updateRecordManagerHeader();
         }
         catch ( Exception e )
         {
@@ -713,7 +715,7 @@ public class RecordManager extends AbstractTransactionManager
 
                 commitCount++;
 
-                if ( commitCount >= spaceReclaimerThreshold )
+                if ( commitCount >= pageReclaimerThreshold )
                 {
                     runReclaimer();
                 }
@@ -760,7 +762,7 @@ public class RecordManager extends AbstractTransactionManager
 
                 commitCount++;
 
-                if ( commitCount >= spaceReclaimerThreshold )
+                if ( commitCount >= pageReclaimerThreshold )
                 {
                     runReclaimer();
                 }
@@ -3801,7 +3803,7 @@ public class RecordManager extends AbstractTransactionManager
      * @param pageIo The page to free
      * @throws IOException If we weren't capable of updating the file
      */
-    private void free( PageIO pageIo ) throws IOException
+    /* no qualifier */ void free( PageIO pageIo ) throws IOException
     {
         freePageLock.lock();
 
@@ -3831,6 +3833,8 @@ public class RecordManager extends AbstractTransactionManager
      */
     /*no qualifier*/ void free( long... offsets ) throws IOException
     {
+        freePageLock.lock();
+
         List<PageIO> pageIos = new ArrayList<PageIO>();
         int pageIndex = 0;
         for ( int i = 0; i < offsets.length; i++ )
@@ -3849,7 +3853,6 @@ public class RecordManager extends AbstractTransactionManager
             }
         }
 
-        freePageLock.lock();
 
         // We add the Page's PageIOs before the
         // existing free pages.
@@ -4079,18 +4082,20 @@ public class RecordManager extends AbstractTransactionManager
      * sets the threshold of the number of commits to be performed before
      * reclaiming the free pages.
      * 
-     * @param spaceReclaimerThreshold the number of commits before the reclaimer runs
+     * @param pageReclaimerThreshold the number of commits before the reclaimer runs
      */
-    /* no qualifier */ void setSpaceReclaimerThreshold( int spaceReclaimerThreshold )
+    /* no qualifier */ void setPageReclaimerThreshold( int pageReclaimerThreshold )
     {
-        this.spaceReclaimerThreshold = spaceReclaimerThreshold;
+        this.pageReclaimerThreshold = pageReclaimerThreshold;
     }
 
+    
     /* no qualifier */ void _disableReclaimer( boolean toggle )
     {
         this.disableReclaimer = toggle;
     }
 
+    
     /**
      * @see Object#toString()
      */
index f618027..8f5625e 100644 (file)
@@ -23,6 +23,10 @@ package org.apache.directory.mavibot.btree;
 import static org.junit.Assert.assertEquals;\r
 \r
 import java.io.File;\r
+import java.io.IOException;\r
+import java.io.RandomAccessFile;\r
+import java.nio.ByteBuffer;\r
+import java.util.ArrayList;\r
 import java.util.Arrays;\r
 import java.util.List;\r
 import java.util.Map;\r
@@ -32,6 +36,7 @@ import java.util.concurrent.CountDownLatch;
 \r
 import org.apache.directory.mavibot.btree.serializer.IntSerializer;\r
 import org.apache.directory.mavibot.btree.serializer.StringSerializer;\r
+import org.apache.directory.mavibot.btree.util.Strings;\r
 import org.junit.After;\r
 import org.junit.Before;\r
 import org.junit.Rule;\r
@@ -65,9 +70,9 @@ public class PageReclaimerTest
         \r
         dbFile = tmpDir.newFile( "spacereclaimer.db" );\r
 \r
-        System.out.println(dbFile.getAbsolutePath());\r
+        //System.out.println(dbFile.getAbsolutePath());\r
         rm = new RecordManager( dbFile.getAbsolutePath() );\r
-        rm.setSpaceReclaimerThreshold( 10 );\r
+        rm.setPageReclaimerThreshold( 10 );\r
         \r
         uidTree = ( PersistedBTree<Integer, String> ) rm.addBTree( TREE_NAME, IntSerializer.INSTANCE, StringSerializer.INSTANCE, false );\r
     }\r
@@ -95,16 +100,15 @@ public class PageReclaimerTest
     public void testReclaimer() throws Exception\r
     {\r
         int total = 11;\r
-        System.out.println( dbFile.length() );\r
         for ( int i=0; i < total; i++ )\r
         {\r
             uidTree.insert( i, String.valueOf( i ) );\r
         }\r
 \r
-        System.out.println( "Total size before closing " + dbFile.length() );\r
-        System.out.println( dbFile.length() );\r
+        //System.out.println( "Total size before closing " + dbFile.length() );\r
+        //System.out.println( dbFile.length() );\r
         closeAndReopenRM();\r
-        System.out.println( "Total size AFTER closing " + dbFile.length() );\r
+        //System.out.println( "Total size AFTER closing " + dbFile.length() );\r
         \r
         int count = 0;\r
         TupleCursor<Integer, String> cursor = uidTree.browse();\r
@@ -133,7 +137,7 @@ public class PageReclaimerTest
     @Test\r
     public void testReclaimerWithMagicNum() throws Exception\r
     {\r
-       rm.setSpaceReclaimerThreshold( 10 );\r
+       rm.setPageReclaimerThreshold( 10 );\r
        \r
         int total = 1120;\r
         for ( int i=0; i < total; i++ )\r
@@ -217,9 +221,9 @@ public class PageReclaimerTest
         \r
         latch.await();\r
         \r
-        System.out.println( "Total size before closing " + dbFile.length() );\r
+        //System.out.println( "Total size before closing " + dbFile.length() );\r
         closeAndReopenRM();\r
-        System.out.println( "Total size AFTER closing " + dbFile.length() );\r
+        //System.out.println( "Total size AFTER closing " + dbFile.length() );\r
         \r
         int count = 0;\r
         TupleCursor<Integer, String> cursor = uidTree.browse();\r
@@ -236,11 +240,22 @@ public class PageReclaimerTest
     }\r
 \r
     @Test\r
+    @SuppressWarnings("all")\r
     public void testInspectTreeState() throws Exception\r
     {\r
         File file = File.createTempFile( "freepagedump", ".db" );\r
+        \r
+        if ( file.exists() )\r
+        {\r
+            boolean deleted = file.delete();\r
+            if ( !deleted )\r
+            {\r
+                throw new IllegalStateException( "Could not delete the data file " + file.getAbsolutePath() );\r
+            }\r
+        }\r
+            \r
         RecordManager manager = new RecordManager( file.getAbsolutePath() );\r
-        manager.setSpaceReclaimerThreshold(17);\r
+        manager.setPageReclaimerThreshold(17);\r
         //manager._disableReclaimer( true );\r
         \r
         PersistedBTreeConfiguration config = new PersistedBTreeConfiguration();\r
@@ -260,34 +275,61 @@ public class PageReclaimerTest
             btree.insert( i, String.valueOf( i ) );\r
         }\r
         \r
+        /*\r
         System.out.println( "Total number of pages created " + manager.nbCreatedPages );\r
         System.out.println( "Total number of pages reused " + manager.nbReusedPages );\r
         System.out.println( "Total number of pages freed " + manager.nbFreedPages );\r
         System.out.println( "Total file size (bytes) " + file.length() );\r
+        */\r
         \r
         long totalPages = file.length() / RecordManager.DEFAULT_PAGE_SIZE;\r
         \r
         // in RM the header page gets skipped before incrementing nbCreatedPages \r
-        //assertEquals( manager.nbCreatedPages.get()+1, totalPages );\r
+        assertEquals( manager.nbCreatedPages.get() + 1, totalPages );\r
+        \r
+        //System.out.println(btree.getRootPage());\r
+        //System.out.println( file.getAbsolutePath() );\r
         \r
-        System.out.println(btree.getRootPage());\r
-        System.out.println( file.getAbsolutePath() );\r
+        check( manager, btree );\r
         \r
+        manager.close();\r
+        \r
+        file.delete();\r
+    }\r
+    \r
+   \r
+    private void check(RecordManager manager, BTree btree) throws Exception\r
+    {\r
         MavibotInspector.check(manager);\r
-        List<Long> lst = MavibotInspector.getFreePages();\r
-        System.out.println(lst);\r
         \r
-        lst = MavibotInspector.getGlobalPages();\r
-        System.out.println(lst);\r
-        System.out.println("Total global offsets " + lst.size() );\r
+        List<Long> allOffsets = MavibotInspector.getGlobalPages();\r
+        //System.out.println( "Global: " + allOffsets);\r
+        //System.out.println("Total global offsets " + allOffsets.size() );\r
         \r
-        lst = MavibotInspector.getPageOffsets( RecordManager.BTREE_OF_BTREES_NAME );\r
-        System.out.println(lst);\r
+        int pagesize = RecordManager.DEFAULT_PAGE_SIZE;\r
+        long total = manager.fileChannel.size();\r
         \r
-        lst = MavibotInspector.getPageOffsets( RecordManager.COPIED_PAGE_BTREE_NAME );\r
-        System.out.println(lst);\r
+        List<Long> unaccounted = new ArrayList<Long>();\r
+        \r
+        for(long i = pagesize; i<= total-pagesize; i+=pagesize)\r
+        {\r
+            if( !allOffsets.contains( Long.valueOf( i ) ) )\r
+            {\r
+                unaccounted.add( i );\r
+            }\r
+        }\r
+        \r
+        TupleCursor<NameRevision, Long> cursor = manager.btreeOfBtrees.browse();\r
+        while(cursor.hasNext())\r
+        {\r
+            Tuple<NameRevision, Long> t = cursor.next();\r
+            System.out.println( t.getKey() + " offset " + t.getValue() );\r
+        }\r
+        \r
+        cursor.close();\r
 \r
-        manager.close();\r
+        //System.out.println("Unaccounted offsets " + unaccounted);\r
+        assertEquals( 0, unaccounted.size() );\r
     }\r
     \r
 }\r