[OAK-8512] Without prefetch CopyOnRead opens index files remotely even if they are locally available

• Type: Bug
• Status: Open
• Priority: Minor
• Resolution: Unresolved
• Component/s: lucene
• Assignee: Unassigned
• Reporter: Vikas Saurabh
• Created: 29/Jul/19 05:36
• Updated: 29/Jul/19 07:18

Description

Without prefetch enabled, CopyOnReadDirectory schedules a copy of file when Directory#openInput is invoked. This happens even if there might a copy of file completely available already (maybe due to CopyOnWriteDirectory). While the scheduled copy would check if the file is valid already but by that time often index file are accessed remotely.

The fix is fairly simple - don't schedule if file exists locally and passes our weak test that length matches what we expect [0].

But, since we strongly advise to enable prefetch, the priority of the issue is minor. Perf impact would be significant during application start so sev should high I guess.

[0]:

$ git diff oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/directory/CopyOnReadDirectory.java
diff --git a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/directory/CopyOnReadDirectory.java b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/directory/CopyOnReadDirectory.java
index b6b16286d2..d49e5d9ea7 100644
--- a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/directory/CopyOnReadDirectory.java
+++ b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/directory/CopyOnReadDirectory.java
@@ -126,11 +126,12 @@ public class CopyOnReadDirectory extends FilterDirectory {
         CORFileReference toPut = new CORFileReference(name);
         CORFileReference old = files.putIfAbsent(name, toPut);
         if (old == null) {
-            log.trace("[{}] scheduled local copy for {}", indexPath, name);
-            copy(toPut);
+            if (copy(toPut)) {
+                log.trace("[{}] scheduled local copy for {}", indexPath, name);
+            }
         }
 
-        //If immediate executor is used the result would be ready right away
+        //If immediate executor is used OR local file already existed, the result would be ready right away
         if (toPut.isLocalValid()) {
             log.trace("[{}] opening new local file {}", indexPath, name);
             return toPut.openLocalInput(context);
@@ -145,7 +146,21 @@ public class CopyOnReadDirectory extends FilterDirectory {
         return local;
     }
 
-    private void copy(final CORFileReference reference) {
+    private boolean copy(final CORFileReference reference) {
+        // quick sanity check
+        if (reference.isLocalValid()) {
+            return false;
+        }
+        try {
+            String name = reference.name;
+            if (local.fileExists(name) && local.fileLength(name) == remote.fileLength(name)) {
+                reference.markValid();
+                return false;
+            }
+        } catch (IOException e) {
+            // ignore
+        }
+
         indexCopier.scheduledForCopy();
         executor.execute(new Runnable() {
             @Override
@@ -154,6 +169,8 @@ public class CopyOnReadDirectory extends FilterDirectory {
                 copyFilesToLocal(reference, true, true);
             }
         });
+
+        return true;
     }

Issue Links

• is related to: OAK-8513 Concurrent index access via CopyOnRead directory can lead to reading directly off of remote