From 2c510f0689b34e71db91f14fba3f03463c379e24 Mon Sep 17 00:00:00 2001
From: Simon Willnauer <simonw@apache.org>
Date: Sun, 26 Apr 2015 15:13:06 +0200
Subject: [PATCH] Allow double-closing of FSTranslog

the translog might be reused across engines which is currently a problem
in the design such that we have to allow calls to `close` more than once.
This moves the closed check for snapshot on the actual file to exit the loop.

Relates to #10807
---
 .../translog/fs/BufferingFsTranslogFile.java  |  5 ++++
 .../index/translog/fs/FsTranslog.java         | 28 +++++++++----------
 .../index/translog/fs/FsTranslogFile.java     |  2 ++
 .../translog/fs/SimpleFsTranslogFile.java     |  6 ++++
 .../translog/AbstractSimpleTranslogTests.java |  2 +-
 5 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/src/main/java/org/elasticsearch/index/translog/fs/BufferingFsTranslogFile.java b/src/main/java/org/elasticsearch/index/translog/fs/BufferingFsTranslogFile.java
index 6fb3988b829..ebd5e125353 100644
--- a/src/main/java/org/elasticsearch/index/translog/fs/BufferingFsTranslogFile.java
+++ b/src/main/java/org/elasticsearch/index/translog/fs/BufferingFsTranslogFile.java
@@ -239,6 +239,11 @@ public class BufferingFsTranslogFile implements FsTranslogFile {
         return channelReference.file();
     }
 
+    @Override
+    public boolean closed() {
+        return this.closed.get();
+    }
+
     class WrapperOutputStream extends OutputStream {
 
         @Override
diff --git a/src/main/java/org/elasticsearch/index/translog/fs/FsTranslog.java b/src/main/java/org/elasticsearch/index/translog/fs/FsTranslog.java
index 757655fa248..58ce5ab5807 100644
--- a/src/main/java/org/elasticsearch/index/translog/fs/FsTranslog.java
+++ b/src/main/java/org/elasticsearch/index/translog/fs/FsTranslog.java
@@ -94,8 +94,6 @@ public class FsTranslog extends AbstractIndexShardComponent implements Translog
 
     private final ApplySettings applySettings = new ApplySettings();
 
-    private final AtomicBoolean closed = new AtomicBoolean(false);
-
     @Inject
     public FsTranslog(ShardId shardId, @IndexSettings Settings indexSettings, IndexSettingsService indexSettingsService,
                       BigArrays bigArrays, ShardPath shardPath) throws IOException {
@@ -141,16 +139,14 @@ public class FsTranslog extends AbstractIndexShardComponent implements Translog
 
     @Override
     public void close() throws IOException {
-        if (closed.compareAndSet(false, true)) {
-            if (indexSettingsService != null) {
-                indexSettingsService.removeListener(applySettings);
-            }
-            rwl.writeLock().lock();
-            try {
-                IOUtils.close(this.trans, this.current);
-            } finally {
-                rwl.writeLock().unlock();
-            }
+        if (indexSettingsService != null) {
+            indexSettingsService.removeListener(applySettings);
+        }
+        rwl.writeLock().lock();
+        try {
+            IOUtils.close(this.trans, this.current);
+        } finally {
+            rwl.writeLock().unlock();
         }
     }
 
@@ -358,13 +354,15 @@ public class FsTranslog extends AbstractIndexShardComponent implements Translog
     @Override
     public FsChannelSnapshot snapshot() throws TranslogException {
         while (true) {
-            if (closed.get()) {
-                throw new TranslogException(shardId, "translog is already closed");
-            }
+            FsTranslogFile current = this.current;
             FsChannelSnapshot snapshot = current.snapshot();
             if (snapshot != null) {
                 return snapshot;
             }
+            if (current.closed() && this.current == current) {
+                // check if we are closed and if we are still current - then this translog is closed and we can exit
+                throw new TranslogException(shardId, "current translog is already closed");
+            }
             Thread.yield();
         }
     }
diff --git a/src/main/java/org/elasticsearch/index/translog/fs/FsTranslogFile.java b/src/main/java/org/elasticsearch/index/translog/fs/FsTranslogFile.java
index 7cfe8744660..a6539847c45 100644
--- a/src/main/java/org/elasticsearch/index/translog/fs/FsTranslogFile.java
+++ b/src/main/java/org/elasticsearch/index/translog/fs/FsTranslogFile.java
@@ -82,4 +82,6 @@ public interface FsTranslogFile extends Closeable {
     TranslogStream getStream();
 
     public Path getPath();
+
+    public boolean closed();
 }
diff --git a/src/main/java/org/elasticsearch/index/translog/fs/SimpleFsTranslogFile.java b/src/main/java/org/elasticsearch/index/translog/fs/SimpleFsTranslogFile.java
index d4d508b83e2..199847d0779 100644
--- a/src/main/java/org/elasticsearch/index/translog/fs/SimpleFsTranslogFile.java
+++ b/src/main/java/org/elasticsearch/index/translog/fs/SimpleFsTranslogFile.java
@@ -182,4 +182,10 @@ public class SimpleFsTranslogFile implements FsTranslogFile {
     public void updateBufferSize(int bufferSize) throws TranslogException {
         // nothing to do here...
     }
+
+    @Override
+    public boolean closed() {
+        return this.closed.get();
+    }
+
 }
diff --git a/src/test/java/org/elasticsearch/index/translog/AbstractSimpleTranslogTests.java b/src/test/java/org/elasticsearch/index/translog/AbstractSimpleTranslogTests.java
index 9b45c4de10b..1a5aa984455 100644
--- a/src/test/java/org/elasticsearch/index/translog/AbstractSimpleTranslogTests.java
+++ b/src/test/java/org/elasticsearch/index/translog/AbstractSimpleTranslogTests.java
@@ -340,7 +340,7 @@ public abstract class AbstractSimpleTranslogTests extends ElasticsearchTestCase
             Translog.Snapshot snapshot = translog.snapshot();
             fail("translog is closed");
         } catch (TranslogException ex) {
-            assertEquals(ex.getMessage(), "translog is already closed");
+            assertEquals(ex.getMessage(), "current translog is already closed");
         }
     }