o fixed an issue of missing pages after PageReclaimer runs by calling updateRecordManagerHeader() after reclaimer.reclaim() in runReclaimer() method
o removed explicit transaction calls inside reclaim() method
o made some methods default protected in RM
o updated tests

diff --git a/mavibot/src/main/java/org/apache/directory/mavibot/btree/PageReclaimer.java b/mavibot/src/main/java/org/apache/directory/mavibot/btree/PageReclaimer.java
index a140c8a..c2bfe38 100644
--- a/mavibot/src/main/java/org/apache/directory/mavibot/btree/PageReclaimer.java
+++ b/mavibot/src/main/java/org/apache/directory/mavibot/btree/PageReclaimer.java
@@ -21,6 +21,7 @@
 
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
@@ -97,9 +98,7 @@
                 // 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 @@
 
                     //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;
diff --git a/mavibot/src/main/java/org/apache/directory/mavibot/btree/RecordManager.java b/mavibot/src/main/java/org/apache/directory/mavibot/btree/RecordManager.java
index fab3e59..3b2fd1e 100644
--- a/mavibot/src/main/java/org/apache/directory/mavibot/btree/RecordManager.java
+++ b/mavibot/src/main/java/org/apache/directory/mavibot/btree/RecordManager.java
@@ -218,7 +218,7 @@
     /** 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 @@
         {
             commitCount = 0;
             reclaimer.reclaim();
+            // must update the headers after reclaim operation
+            updateRecordManagerHeader();
         }
         catch ( Exception e )
         {
@@ -713,7 +715,7 @@
 
                 commitCount++;
 
-                if ( commitCount >= spaceReclaimerThreshold )
+                if ( commitCount >= pageReclaimerThreshold )
                 {
                     runReclaimer();
                 }
@@ -760,7 +762,7 @@
 
                 commitCount++;
 
-                if ( commitCount >= spaceReclaimerThreshold )
+                if ( commitCount >= pageReclaimerThreshold )
                 {
                     runReclaimer();
                 }
@@ -3801,7 +3803,7 @@
      * @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 @@
      */
     /*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 @@
             }
         }
 
-        freePageLock.lock();
 
         // We add the Page's PageIOs before the
         // existing free pages.
@@ -4079,18 +4082,20 @@
      * 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()
      */
diff --git a/mavibot/src/test/java/org/apache/directory/mavibot/btree/PageReclaimerTest.java b/mavibot/src/test/java/org/apache/directory/mavibot/btree/PageReclaimerTest.java
index f618027..8f5625e 100644
--- a/mavibot/src/test/java/org/apache/directory/mavibot/btree/PageReclaimerTest.java
+++ b/mavibot/src/test/java/org/apache/directory/mavibot/btree/PageReclaimerTest.java
@@ -23,6 +23,10 @@
 import static org.junit.Assert.assertEquals;

 

 import java.io.File;

+import java.io.IOException;

+import java.io.RandomAccessFile;

+import java.nio.ByteBuffer;

+import java.util.ArrayList;

 import java.util.Arrays;

 import java.util.List;

 import java.util.Map;

@@ -32,6 +36,7 @@
 

 import org.apache.directory.mavibot.btree.serializer.IntSerializer;

 import org.apache.directory.mavibot.btree.serializer.StringSerializer;

+import org.apache.directory.mavibot.btree.util.Strings;

 import org.junit.After;

 import org.junit.Before;

 import org.junit.Rule;

@@ -65,9 +70,9 @@
         

         dbFile = tmpDir.newFile( "spacereclaimer.db" );

 

-        System.out.println(dbFile.getAbsolutePath());

+        //System.out.println(dbFile.getAbsolutePath());

         rm = new RecordManager( dbFile.getAbsolutePath() );

-        rm.setSpaceReclaimerThreshold( 10 );

+        rm.setPageReclaimerThreshold( 10 );

         

         uidTree = ( PersistedBTree<Integer, String> ) rm.addBTree( TREE_NAME, IntSerializer.INSTANCE, StringSerializer.INSTANCE, false );

     }

@@ -95,16 +100,15 @@
     public void testReclaimer() throws Exception

     {

         int total = 11;

-        System.out.println( dbFile.length() );

         for ( int i=0; i < total; i++ )

         {

             uidTree.insert( i, String.valueOf( i ) );

         }

 

-        System.out.println( "Total size before closing " + dbFile.length() );

-        System.out.println( dbFile.length() );

+        //System.out.println( "Total size before closing " + dbFile.length() );

+        //System.out.println( dbFile.length() );

         closeAndReopenRM();

-        System.out.println( "Total size AFTER closing " + dbFile.length() );

+        //System.out.println( "Total size AFTER closing " + dbFile.length() );

         

         int count = 0;

         TupleCursor<Integer, String> cursor = uidTree.browse();

@@ -133,7 +137,7 @@
     @Test

     public void testReclaimerWithMagicNum() throws Exception

     {

-    	rm.setSpaceReclaimerThreshold( 10 );

+    	rm.setPageReclaimerThreshold( 10 );

     	

         int total = 1120;

         for ( int i=0; i < total; i++ )

@@ -217,9 +221,9 @@
         

         latch.await();

         

-        System.out.println( "Total size before closing " + dbFile.length() );

+        //System.out.println( "Total size before closing " + dbFile.length() );

         closeAndReopenRM();

-        System.out.println( "Total size AFTER closing " + dbFile.length() );

+        //System.out.println( "Total size AFTER closing " + dbFile.length() );

         

         int count = 0;

         TupleCursor<Integer, String> cursor = uidTree.browse();

@@ -236,11 +240,22 @@
     }

 

     @Test

+    @SuppressWarnings("all")

     public void testInspectTreeState() throws Exception

     {

         File file = File.createTempFile( "freepagedump", ".db" );

+        

+        if ( file.exists() )

+        {

+            boolean deleted = file.delete();

+            if ( !deleted )

+            {

+                throw new IllegalStateException( "Could not delete the data file " + file.getAbsolutePath() );

+            }

+        }

+            

         RecordManager manager = new RecordManager( file.getAbsolutePath() );

-        manager.setSpaceReclaimerThreshold(17);

+        manager.setPageReclaimerThreshold(17);

         //manager._disableReclaimer( true );

         

         PersistedBTreeConfiguration config = new PersistedBTreeConfiguration();

@@ -260,34 +275,61 @@
             btree.insert( i, String.valueOf( i ) );

         }

         

+        /*

         System.out.println( "Total number of pages created " + manager.nbCreatedPages );

         System.out.println( "Total number of pages reused " + manager.nbReusedPages );

         System.out.println( "Total number of pages freed " + manager.nbFreedPages );

         System.out.println( "Total file size (bytes) " + file.length() );

+        */

         

         long totalPages = file.length() / RecordManager.DEFAULT_PAGE_SIZE;

         

         // in RM the header page gets skipped before incrementing nbCreatedPages 

-        //assertEquals( manager.nbCreatedPages.get()+1, totalPages );

+        assertEquals( manager.nbCreatedPages.get() + 1, totalPages );

         

-        System.out.println(btree.getRootPage());

-        System.out.println( file.getAbsolutePath() );

+        //System.out.println(btree.getRootPage());

+        //System.out.println( file.getAbsolutePath() );

         

-        MavibotInspector.check(manager);

-        List<Long> lst = MavibotInspector.getFreePages();

-        System.out.println(lst);

+        check( manager, btree );

         

-        lst = MavibotInspector.getGlobalPages();

-        System.out.println(lst);

-        System.out.println("Total global offsets " + lst.size() );

-        

-        lst = MavibotInspector.getPageOffsets( RecordManager.BTREE_OF_BTREES_NAME );

-        System.out.println(lst);

-        

-        lst = MavibotInspector.getPageOffsets( RecordManager.COPIED_PAGE_BTREE_NAME );

-        System.out.println(lst);

-

         manager.close();

+        

+        file.delete();

+    }

+    

+   

+    private void check(RecordManager manager, BTree btree) throws Exception

+    {

+        MavibotInspector.check(manager);

+        

+        List<Long> allOffsets = MavibotInspector.getGlobalPages();

+        //System.out.println( "Global: " + allOffsets);

+        //System.out.println("Total global offsets " + allOffsets.size() );

+        

+        int pagesize = RecordManager.DEFAULT_PAGE_SIZE;

+        long total = manager.fileChannel.size();

+        

+        List<Long> unaccounted = new ArrayList<Long>();

+        

+        for(long i = pagesize; i<= total-pagesize; i+=pagesize)

+        {

+            if( !allOffsets.contains( Long.valueOf( i ) ) )

+            {

+                unaccounted.add( i );

+            }

+        }

+        

+        TupleCursor<NameRevision, Long> cursor = manager.btreeOfBtrees.browse();

+        while(cursor.hasNext())

+        {

+            Tuple<NameRevision, Long> t = cursor.next();

+            System.out.println( t.getKey() + " offset " + t.getValue() );

+        }

+        

+        cursor.close();

+

+        //System.out.println("Unaccounted offsets " + unaccounted);

+        assertEquals( 0, unaccounted.size() );

     }

     

 }