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

[am...@apache.org]

Author: amitj
Date: Mon Mar 12 10:23:57 2018
New Revision: 1826532

URL: http://svn.apache.org/viewvc?rev=1826532&view=rev
Log:
OAK-7209: Race condition can resurrect blobs during blob GC

Calling globalMerge() to retrieve and merge locally all blob ids files from DataStore
Credits to Csaba Varga for the patch

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

Modified: jackrabbit/oak/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/BlobIdTracker.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/BlobIdTracker.java?rev=1826532&r1=1826531&r2=1826532&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/BlobIdTracker.java (original)
+++ jackrabbit/oak/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/BlobIdTracker.java Mon Mar 12 10:23:57 2018
@@ -144,8 +144,8 @@ public class BlobIdTracker implements Cl
     }
 
     @Override public void remove(File recs, Options options) throws IOException {
+        globalMerge();
         if (options == Options.ACTIVE_DELETION) {
-            globalMerge();
             deleteTracker.track(recs);
         }
         store.removeRecords(recs);
@@ -153,11 +153,13 @@ public class BlobIdTracker implements Cl
     }
 
     @Override public void remove(File recs) throws IOException {
+        globalMerge();
         store.removeRecords(recs);
         snapshot(true);
     }
 
     @Override public void remove(Iterator<String> recs) throws IOException {
+        globalMerge();
         store.removeRecords(recs);
         snapshot(true);
     }

Modified: jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/BlobIdTrackerTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/BlobIdTrackerTest.java?rev=1826532&r1=1826531&r2=1826532&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/BlobIdTrackerTest.java (original)
+++ jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/BlobIdTrackerTest.java Mon Mar 12 10:23:57 2018
@@ -124,7 +124,7 @@ public class BlobIdTrackerTest {
     @Test
     public void addSnapshotRemove() throws Exception {
         LOG.info("In addSnapshotRemove");
-        snapshotRemove(tracker.new SnapshotJob());
+        snapshotRemove(tracker.new SnapshotJob(), false);
     }
 
     @Test
@@ -132,7 +132,7 @@ public class BlobIdTrackerTest {
         LOG.info("In snapshotIgnoreAfterRemove");
         BlobIdTracker.SnapshotJob job = tracker.new SnapshotJob();
 
-        snapshotRemove(job);
+        snapshotRemove(job, false);
 
         // Since already retrieved the datastore should be empty unless the snapshot has actually run
         ScheduledFuture<?> scheduledFuture =
@@ -149,7 +149,8 @@ public class BlobIdTrackerTest {
         Clock clock = Clock.ACCURATE;
         BlobIdTracker.SnapshotJob job = tracker.new SnapshotJob(100, clock);
 
-        Set<String> present = snapshotRemove(job);
+        // Mimics a call to get after add and before remove similar to the calls in GC
+        Set<String> present = snapshotRemove(job, false);
 
         clock.waitUntil(System.currentTimeMillis() + 100);
 
@@ -162,12 +163,36 @@ public class BlobIdTrackerTest {
             read(dataStore.getAllMetadataRecords(BLOBREFERENCES.getType())));
     }
 
-    private Set<String> snapshotRemove(BlobIdTracker.SnapshotJob job) throws Exception {
+    @Test
+    public void snapshotBeforeRemove() throws Exception {
+        LOG.info("In snapshotBeforeRemove");
+
+        Clock clock = Clock.ACCURATE;
+        BlobIdTracker.SnapshotJob job = tracker.new SnapshotJob(100, clock);
+
+        //Mimic an intervening snapshot between add and remove by skipping the retrieve call.
+        Set<String> present = snapshotRemove(job, true);
+
+        clock.waitUntil(System.currentTimeMillis() + 100);
+
+        // Since already retrieved the datastore should not be empty unless the snapshot is ignored
+        ScheduledFuture<?> scheduledFuture =
+            scheduler.schedule(job, 0, TimeUnit.MILLISECONDS);
+        scheduledFuture.get();
+
+        assertEquals("Elements not equal after snapshot after remove", present,
+            read(dataStore.getAllMetadataRecords(BLOBREFERENCES.getType())));
+    }
+
+    private Set<String> snapshotRemove(BlobIdTracker.SnapshotJob job, boolean skipGetBeforeRemove) throws Exception {
         Set<String> initAdd = add(tracker, range(0, 4));
         ScheduledFuture<?> scheduledFuture =
             scheduler.schedule(job, 0, TimeUnit.MILLISECONDS);
         scheduledFuture.get();
-        assertEquals("Extra elements after add", initAdd, retrieve(tracker));
+
+        if (!skipGetBeforeRemove) {
+            assertEquals("Extra elements after add", initAdd, retrieve(tracker));
+        }
 
         remove(tracker, folder.newFile(), initAdd, range(1, 2));
         assertEquals("Extra elements after removes synced with datastore",