Create cache files with CREATE_NEW & SPARSE options (#79371)

* Create cache files with CREATE_NEW & SPARSE options * assert + doc * rename
elastic · Oct 19, 2021 · a3f057c · a3f057c
1 parent 5402f4c
commit a3f057c
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 9 deletions.
diff --git a/...ots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFile.java b/...ots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFile.java
@@ -53,12 +53,14 @@ public interface ModificationListener {
         void onCacheFileDelete(CacheFile cacheFile);
     }
 
-    private static final StandardOpenOption[] OPEN_OPTIONS = new StandardOpenOption[] {
+    private static final StandardOpenOption[] CREATE_OPTIONS = new StandardOpenOption[] {
         StandardOpenOption.READ,
         StandardOpenOption.WRITE,
-        StandardOpenOption.CREATE,
+        StandardOpenOption.CREATE_NEW,
         StandardOpenOption.SPARSE };
 
+    private static final StandardOpenOption[] OPEN_OPTIONS = new StandardOpenOption[] { StandardOpenOption.READ, StandardOpenOption.WRITE };
+
     /**
      * Reference counter that counts the number of eviction listeners referencing this cache file plus the number of open file channels
      * for it. Once this instance has been evicted, all listeners notified and all {@link FileChannelReference} for it released,
@@ -100,8 +102,8 @@ private final class FileChannelReference extends AbstractRefCounted {
 
         private final FileChannel fileChannel;
 
-        FileChannelReference() throws IOException {
-            this.fileChannel = FileChannel.open(file, OPEN_OPTIONS);
+        FileChannelReference(StandardOpenOption[] options) throws IOException {
+            this.fileChannel = FileChannel.open(file, options);
             refCounter.incRef();
         }
 
@@ -124,19 +126,26 @@ protected void closeInternal() {
     @Nullable
     private volatile FileChannelReference channelRef;
 
+    /**
+     * {@code true} if the physical cache file exists on disk
+     */
+    private volatile boolean fileExists;
+
     public CacheFile(CacheKey cacheKey, long length, Path file, ModificationListener listener) {
-        this(cacheKey, new SparseFileTracker(file.toString(), length), file, listener);
+        this(cacheKey, new SparseFileTracker(file.toString(), length), file, listener, false);
     }
 
     public CacheFile(CacheKey cacheKey, long length, Path file, SortedSet<ByteRange> ranges, ModificationListener listener) {
-        this(cacheKey, new SparseFileTracker(file.toString(), length, ranges), file, listener);
+        this(cacheKey, new SparseFileTracker(file.toString(), length, ranges), file, listener, true);
     }
 
-    private CacheFile(CacheKey cacheKey, SparseFileTracker tracker, Path file, ModificationListener listener) {
+    private CacheFile(CacheKey cacheKey, SparseFileTracker tracker, Path file, ModificationListener listener, boolean fileExists) {
         this.cacheKey = Objects.requireNonNull(cacheKey);
         this.tracker = Objects.requireNonNull(tracker);
         this.file = Objects.requireNonNull(file);
         this.listener = Objects.requireNonNull(listener);
+        assert fileExists == Files.exists(file) : file + " exists? " + fileExists;
+        this.fileExists = fileExists;
         assert invariant();
     }
 
@@ -182,7 +191,8 @@ public void acquire(final EvictionListener listener) throws IOException {
                     ensureOpen();
                     if (listeners.isEmpty()) {
                         assert channelRef == null;
-                        channelRef = new FileChannelReference();
+                        channelRef = new FileChannelReference(fileExists ? OPEN_OPTIONS : CREATE_OPTIONS);
+                        fileExists = true;
                     }
                     final boolean added = listeners.add(listener);
                     assert added : "listener already exists " + listener;

diff --git a/...rc/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java b/...rc/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java
@@ -58,14 +58,15 @@ public void onCacheFileDelete(CacheFile cacheFile) {}
     private static final CacheKey CACHE_KEY = new CacheKey("_snap_uuid", "_snap_index", new ShardId("_name", "_uuid", 0), "_filename");
 
     public void testGetCacheKey() throws Exception {
+        final Path file = createTempDir().resolve("file.new");
         final CacheKey cacheKey = new CacheKey(
             UUIDs.randomBase64UUID(random()),
             randomAlphaOfLength(5).toLowerCase(Locale.ROOT),
             new ShardId(randomAlphaOfLength(5).toLowerCase(Locale.ROOT), UUIDs.randomBase64UUID(random()), randomInt(5)),
             randomAlphaOfLength(105).toLowerCase(Locale.ROOT)
         );
 
-        final CacheFile cacheFile = new CacheFile(cacheKey, randomLongBetween(1, 100), createTempFile(), NOOP);
+        final CacheFile cacheFile = new CacheFile(cacheKey, randomLongBetween(1, 100), file, NOOP);
         assertThat(cacheFile.getCacheKey(), sameInstance(cacheKey));
     }