diff --git a/core/src/main/java/org/elasticsearch/env/NodeEnvironment.java b/core/src/main/java/org/elasticsearch/env/NodeEnvironment.java index 91cb96d60d7..ecebe411534 100644 --- a/core/src/main/java/org/elasticsearch/env/NodeEnvironment.java +++ b/core/src/main/java/org/elasticsearch/env/NodeEnvironment.java @@ -901,11 +901,12 @@ public final class NodeEnvironment implements Closeable { final NodePath[] nodePaths = nodePaths(); for (NodePath nodePath : nodePaths) { assert Files.isDirectory(nodePath.path) : nodePath.path + " is not a directory"; - final Path src = nodePath.path.resolve("__es__.tmp"); - final Path target = nodePath.path.resolve("__es__.final"); + final Path src = nodePath.path.resolve(TEMP_FILE_NAME + ".tmp"); + final Path target = nodePath.path.resolve(TEMP_FILE_NAME + ".final"); try { + Files.deleteIfExists(src); Files.createFile(src); - Files.move(src, target, StandardCopyOption.ATOMIC_MOVE); + Files.move(src, target, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); } catch (AtomicMoveNotSupportedException ex) { throw new IllegalStateException("atomic_move is not supported by the filesystem on path [" + nodePath.path @@ -1005,19 +1006,19 @@ public final class NodeEnvironment implements Closeable { } } + // package private for testing + static final String TEMP_FILE_NAME = ".es_temp_file"; + private static void tryWriteTempFile(Path path) throws IOException { if (Files.exists(path)) { - Path resolve = path.resolve(".es_temp_file"); - boolean tempFileCreated = false; + Path resolve = path.resolve(TEMP_FILE_NAME); try { + // delete any lingering file from a previous failure + Files.deleteIfExists(resolve); Files.createFile(resolve); - tempFileCreated = true; + Files.delete(resolve); } catch (IOException ex) { - throw new IOException("failed to write in data directory [" + path + "] write permission is required", ex); - } finally { - if (tempFileCreated) { - Files.deleteIfExists(resolve); - } + throw new IOException("failed to test writes in data directory [" + path + "] write permission is required", ex); } } } diff --git a/core/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java b/core/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java index f067212caaf..42cb4a5811b 100644 --- a/core/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java +++ b/core/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java @@ -34,6 +34,7 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.IndexSettingsModule; import java.io.IOException; +import java.nio.file.AtomicMoveNotSupportedException; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; @@ -416,6 +417,39 @@ public class NodeEnvironmentTests extends ESTestCase { env.close(); } + public void testExistingTempFiles() throws IOException { + String[] paths = tmpPaths(); + // simulate some previous left over temp files + for (String path : randomSubsetOf(randomIntBetween(1, paths.length), paths)) { + final Path nodePath = NodeEnvironment.resolveNodePath(PathUtils.get(path), 0); + Files.createDirectories(nodePath); + Files.createFile(nodePath.resolve(NodeEnvironment.TEMP_FILE_NAME)); + if (randomBoolean()) { + Files.createFile(nodePath.resolve(NodeEnvironment.TEMP_FILE_NAME + ".tmp")); + } + if (randomBoolean()) { + Files.createFile(nodePath.resolve(NodeEnvironment.TEMP_FILE_NAME + ".final")); + } + } + NodeEnvironment env = newNodeEnvironment(paths, Settings.EMPTY); + try { + env.ensureAtomicMoveSupported(); + } catch (AtomicMoveNotSupportedException e) { + // that's OK :) + } + env.close(); + // check we clean up + for (String path: paths) { + final Path nodePath = NodeEnvironment.resolveNodePath(PathUtils.get(path), 0); + final Path tempFile = nodePath.resolve(NodeEnvironment.TEMP_FILE_NAME); + assertFalse(tempFile + " should have been cleaned", Files.exists(tempFile)); + final Path srcTempFile = nodePath.resolve(NodeEnvironment.TEMP_FILE_NAME + ".src"); + assertFalse(srcTempFile + " should have been cleaned", Files.exists(srcTempFile)); + final Path targetTempFile = nodePath.resolve(NodeEnvironment.TEMP_FILE_NAME + ".target"); + assertFalse(targetTempFile + " should have been cleaned", Files.exists(targetTempFile)); + } + } + /** Converts an array of Strings to an array of Paths, adding an additional child if specified */ private Path[] stringsToPaths(String[] strings, String additional) { Path[] locations = new Path[strings.length]; diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/env/NodeEnvironmentEvilTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/env/NodeEnvironmentEvilTests.java index ccf5e1b105e..3eebf4a2f64 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/env/NodeEnvironmentEvilTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/env/NodeEnvironmentEvilTests.java @@ -30,7 +30,6 @@ import java.nio.file.Path; import java.nio.file.attribute.PosixFileAttributeView; import java.nio.file.attribute.PosixFilePermission; import java.util.Arrays; -import java.util.Collections; import java.util.HashSet; public class NodeEnvironmentEvilTests extends ESTestCase { @@ -75,7 +74,7 @@ public class NodeEnvironmentEvilTests extends ESTestCase { IOException ioException = expectThrows(IOException.class, () -> { new NodeEnvironment(build, new Environment(build)); }); - assertTrue(ioException.getMessage(), ioException.getMessage().startsWith("failed to write in data directory")); + assertTrue(ioException.getMessage(), ioException.getMessage().startsWith("failed to test writes in data directory")); } } @@ -100,7 +99,7 @@ public class NodeEnvironmentEvilTests extends ESTestCase { IOException ioException = expectThrows(IOException.class, () -> { new NodeEnvironment(build, new Environment(build)); }); - assertTrue(ioException.getMessage(), ioException.getMessage().startsWith("failed to write in data directory")); + assertTrue(ioException.getMessage(), ioException.getMessage().startsWith("failed to test writes in data directory")); } } }