You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by am...@apache.org on 2017/01/18 06:25:14 UTC

svn commit: r1779292 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/BlobIdTracker.java test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/DataStoreTrackerGCTest.java

Author: amitj
Date: Wed Jan 18 06:25:14 2017
New Revision: 1779292

URL: http://svn.apache.org/viewvc?rev=1779292&view=rev
Log:
OAK-5461: [BlobGC] BlobIdTracker remove() should merge generations

- Creating a snapshot before removing deleted blob ids
- Guarding snapshot against concurrently run remove (a rare situation)

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/BlobIdTracker.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/DataStoreTrackerGCTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/BlobIdTracker.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/BlobIdTracker.java?rev=1779292&r1=1779291&r2=1779292&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/BlobIdTracker.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/BlobIdTracker.java Wed Jan 18 06:25:14 2017
@@ -312,10 +312,14 @@ public class BlobIdTracker implements Cl
         /* Lock for operations on references file */
         private final ReentrantLock refLock;
 
+        /* Lock for snapshot */
+        private final ReentrantLock snapshotLock;
+
         BlobIdStore(File rootDir, String prefix) throws IOException {
             this.rootDir = rootDir;
             this.prefix = prefix;
             this.refLock = new ReentrantLock();
+            this.snapshotLock = new ReentrantLock();
 
             // Retrieve the process file if it exists
             processFile =
@@ -443,6 +447,9 @@ public class BlobIdTracker implements Cl
          * @throws IOException
          */
         protected void removeRecords(File recs) throws IOException {
+            // do a snapshot
+            snapshot();
+
             refLock.lock();
             try {
                 sort(getBlobRecordsFile());
@@ -520,8 +527,13 @@ public class BlobIdTracker implements Cl
          * @throws IOException
          */
         protected void snapshot() throws IOException {
-            nextGeneration();
-            merge(generations, false);
+            snapshotLock.lock();
+            try {
+                nextGeneration();
+                merge(generations, false);
+            } finally {
+                snapshotLock.unlock();
+            }
         }
 
         @Override

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/DataStoreTrackerGCTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/DataStoreTrackerGCTest.java?rev=1779292&r1=1779291&r2=1779292&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/DataStoreTrackerGCTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/DataStoreTrackerGCTest.java Wed Jan 18 06:25:14 2017
@@ -143,6 +143,13 @@ public class DataStoreTrackerGCTest {
         assertEquals(state.blobsPresent, existingAfterGC);
         // Tracked blobs should reflect deletions after gc
         assertEquals(state.blobsPresent, retrieveTracked(tracker));
+
+        // Create a snapshot
+        ScheduledFuture<?> scheduledFuture = newSingleThreadScheduledExecutor()
+            .schedule(tracker.new SnapshotJob(), 0, MILLISECONDS);
+        scheduledFuture.get();
+        // Tracked blobs should reflect deletions after gc and the deleted should not get resurrected
+        assertEquals(state.blobsPresent, retrieveTracked(tracker));
     }
 
     private HashSet<String> addNodeSpecialChars(DocumentNodeStore ds) throws Exception {
@@ -206,6 +213,13 @@ public class DataStoreTrackerGCTest {
         assertEquals(state.blobsPresent, existingAfterGC);
         // Tracked blobs should reflect deletions after gc and also the additions after
         assertEquals(state.blobsPresent, retrieveTracked(tracker));
+
+        // Create a snapshot
+        scheduledFuture = newSingleThreadScheduledExecutor()
+            .schedule(tracker.new SnapshotJob(), 0, MILLISECONDS);
+        scheduledFuture.get();
+        // Tracked blobs should reflect deletions after gc and the deleted should not get resurrected
+        assertEquals(state.blobsPresent, retrieveTracked(tracker));
     }
 
     @Test