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 ca...@apache.org on 2018/09/12 22:38:45 UTC

svn commit: r1840769 - in /jackrabbit/oak/trunk/oak-lucene/src: main/java/org/apache/jackrabbit/oak/plugins/index/lucene/directory/CopyOnReadDirectory.java test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexCopierCleanupTest.java

Author: catholicon
Date: Wed Sep 12 22:38:45 2018
New Revision: 1840769

URL: http://svn.apache.org/viewvc?rev=1840769&view=rev
Log:
OAK-7751: CopyOnReadDirectory#removeDeletedFiles asks IndexCopier to check timestamp for (remote only) segments.gen leading to failure to clean up local files

Modified:
    jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/directory/CopyOnReadDirectory.java
    jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexCopierCleanupTest.java

Modified: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/directory/CopyOnReadDirectory.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/directory/CopyOnReadDirectory.java?rev=1840769&r1=1840768&r2=1840769&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/directory/CopyOnReadDirectory.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/directory/CopyOnReadDirectory.java Wed Sep 12 22:38:45 2018
@@ -46,6 +46,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import static com.google.common.collect.Maps.newConcurrentMap;
+import static java.util.Arrays.stream;
 import static org.apache.jackrabbit.oak.commons.IOUtils.humanReadableByteCount;
 
 /**
@@ -306,7 +307,9 @@ public class CopyOnReadDirectory extends
     }
 
     private void removeDeletedFiles() throws IOException {
-        Set<String> remoteFiles = ImmutableSet.copyOf(remote.listAll());
+        Set<String> remoteFiles = stream(remote.listAll())
+                .filter(name -> !IndexCopier.REMOTE_ONLY.contains(name))
+                .collect(Collectors.toSet());
 
         long maxTS = IndexCopier.getNewestLocalFSTimestampFor(remoteFiles, local);
         if (maxTS == -1) {

Modified: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexCopierCleanupTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexCopierCleanupTest.java?rev=1840769&r1=1840768&r2=1840769&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexCopierCleanupTest.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexCopierCleanupTest.java Wed Sep 12 22:38:45 2018
@@ -352,6 +352,98 @@ public class IndexCopierCleanupTest {
                 Sets.newHashSet(new SimpleFSDirectory(localFSDir).listAll()));
     }
 
+    @Test
+    public void remoteOnlyFilesDontAvoidDeletion() throws Exception {
+        writeFile(remote, "a");
+        for (String name : IndexCopier.REMOTE_ONLY) {
+            writeFile(remote, name);
+        }
+        remote.close();
+
+        // get files to get locally copied
+        copier.getCoRDir().close();
+
+        remote.deleteFile("a");
+        writeFile(remote, "b");
+        remote.close();
+
+        assertTrue(existsLocally("a"));
+
+        copier.getCoRDir().close();
+
+        assertFalse(existsLocally("a"));
+        assertTrue(existsLocally("b"));
+    }
+
+    @Test
+    public void remoteOnlyFilesIfExistingGetDeleted() throws Exception {
+        Directory cow = copier.getCoWDir();
+        writeFile(cow, "a");
+        for (String name : IndexCopier.REMOTE_ONLY) {
+            writeFile(cow, name);
+        }
+        cow.close();
+
+        remote.deleteFile("a");
+        writeFile(remote, "b");
+        remote.close();
+
+        assertTrue(existsLocally("a"));
+        for (String name : IndexCopier.REMOTE_ONLY) {
+            assertTrue(existsLocally(name));
+        }
+
+        copier.getCoRDir().close();
+
+        assertFalse(existsLocally("a"));
+        for (String name : IndexCopier.REMOTE_ONLY) {
+            assertFalse(existsLocally(name));
+        }
+        assertTrue(existsLocally("b"));
+    }
+
+    @Test
+    public void remoteOnlyFilesNotCleanedIfUpdatedRecently() throws Exception {
+        // Create remote_only files (and a normal one)
+        Directory cow = copier.getCoWDir();
+        writeFile(cow, "a");
+        for (String name : IndexCopier.REMOTE_ONLY) {
+            writeFile(cow, name);
+        }
+        cow.close();
+
+        // delete all existing files and create a new normal one
+        remote.deleteFile("a");
+        for (String name : IndexCopier.REMOTE_ONLY) {
+            remote.deleteFile(name);
+        }
+        writeFile(remote, "b");
+        remote.close();
+
+        // get a CoR at this state (only sees "b" in list of files)
+        Directory cor = copier.getCoRDir();
+
+        // re-create remote_only files
+        cow = copier.getCoWDir();
+        for (String name : IndexCopier.REMOTE_ONLY) {
+            writeFile(cow, name);
+        }
+        cow.close();
+
+        assertTrue(existsLocally("a"));
+        for (String name : IndexCopier.REMOTE_ONLY) {
+            assertTrue(existsLocally(name));
+        }
+
+        cor.close();
+
+        assertFalse(existsLocally("a"));
+        for (String name : IndexCopier.REMOTE_ONLY) {
+            assertTrue(existsLocally(name));
+        }
+        assertTrue(existsLocally("b"));
+    }
+
     private boolean existsLocally(String fileName) {
         return new File(localFSDir, fileName).exists();
     }