Skip to content

Cut over to Path API for file deletion #8366

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 6, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions dev-tools/forbidden/all-signatures.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
@defaultMessage Convert to URI
java.net.URL#getPath()
java.net.URL#getFile()

java.io.File#delete() @ use Files.delete for real exception, IOUtils.deleteFilesIgnoringExceptions if you dont care
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ interface BlobNameFilter {
*/
OutputStream createOutput(String blobName) throws IOException;

boolean deleteBlob(String blobName) throws IOException;
void deleteBlob(String blobName) throws IOException;

void deleteBlobsByPrefix(String blobNamePrefix) throws IOException;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,16 @@
*/
package org.elasticsearch.common.blobstore;

import java.io.IOException;

/**
*
*/
public interface BlobStore {

BlobContainer blobContainer(BlobPath path);

void delete(BlobPath path);
void delete(BlobPath path) throws IOException;

void close();
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
import org.elasticsearch.common.io.FileSystemUtils;

import java.io.*;
import java.nio.file.Files;
import java.nio.file.Path;

/**
*
Expand Down Expand Up @@ -65,8 +67,12 @@ public ImmutableMap<String, BlobMetaData> listBlobs() throws IOException {
return builder.immutableMap();
}

public boolean deleteBlob(String blobName) throws IOException {
return new File(path, blobName).delete();
@Override
public void deleteBlob(String blobName) throws IOException {
Path blobPath = new File(path, blobName).toPath();
if (Files.exists(blobPath)) {
Files.delete(blobPath);
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.common.blobstore.fs;

import org.apache.lucene.util.IOUtils;
import org.elasticsearch.common.blobstore.BlobContainer;
import org.elasticsearch.common.blobstore.BlobPath;
import org.elasticsearch.common.blobstore.BlobStore;
Expand All @@ -30,6 +31,7 @@
import org.elasticsearch.common.unit.ByteSizeValue;

import java.io.File;
import java.io.IOException;

/**
*
Expand Down Expand Up @@ -74,8 +76,8 @@ public BlobContainer blobContainer(BlobPath path) {
}

@Override
public void delete(BlobPath path) {
FileSystemUtils.deleteRecursively(buildPath(path));
public void delete(BlobPath path) throws IOException {
IOUtils.rm(buildPath(path).toPath());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public ImmutableMap<String, BlobMetaData> listBlobs() throws IOException {
* This operation is not supported by URLBlobContainer
*/
@Override
public boolean deleteBlob(String blobName) throws IOException {
public void deleteBlob(String blobName) throws IOException {
throw new UnsupportedOperationException("URL repository is read only");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.apache.lucene.util.IOUtils;
import org.elasticsearch.ElasticsearchTimeoutException;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.io.FileSystemUtils;
import org.elasticsearch.common.unit.TimeValue;

import java.io.*;
Expand Down Expand Up @@ -346,7 +347,7 @@ private boolean downloadFile() throws FileNotFoundException, IOException {
// Try to delete the garbage we'd otherwise leave
// behind.
IOUtils.closeWhileHandlingException(os, is);
dest.delete();
FileSystemUtils.deleteFilesIgnoringExceptions(dest.toPath());
} else {
IOUtils.close(os, is);
}
Expand Down Expand Up @@ -385,7 +386,7 @@ void closeStreams() throws IOException {
} else {
IOUtils.closeWhileHandlingException(is, os);
if (dest != null && dest.exists()) {
dest.delete();
FileSystemUtils.deleteFilesIgnoringExceptions(dest.toPath());
}
}
}
Expand Down
100 changes: 29 additions & 71 deletions src/main/java/org/elasticsearch/common/io/FileSystemUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -78,84 +78,34 @@ public static boolean exists(File... files) {
}

/**
* Deletes the given files recursively. if <tt>deleteRoots</tt> is set to <code>true</code>
* the given root files will be deleted as well. Otherwise only their content is deleted.
* Returns an array of {@link Path} build from the correspondent element
* in the input array using {@link java.io.File#toPath()}
* @param files the files to get paths for
*/
public static boolean deleteRecursively(File[] roots, boolean deleteRoots) {

boolean deleted = true;
for (File root : roots) {
deleted &= deleteRecursively(root, deleteRoots);
public static Path[] toPaths(File... files) {
Path[] paths = new Path[files.length];
for (int i = 0; i < files.length; i++) {
paths[i] = files[i].toPath();
}
return deleted;
return paths;
}

/**
* Deletes all subdirectories of the given roots recursively.
* Deletes all subdirectories in the given path recursively
* @throws java.lang.IllegalArgumentException if the given path is not a directory
*/
public static boolean deleteSubDirectories(File[] roots) {

boolean deleted = true;
for (File root : roots) {
if (root.isDirectory()) {
File[] files = root.listFiles(new FileFilter() {
@Override
public boolean accept(File pathname) {
return pathname.isDirectory();
}
});
deleted &= deleteRecursively(files, true);
}

}
return deleted;
}

/**
* Deletes the given files recursively including the given roots.
*/
public static boolean deleteRecursively(File... roots) {
return deleteRecursively(roots, true);
}

/**
* Delete the supplied {@link java.io.File} - for directories,
* recursively delete any nested directories or files as well.
*
* @param root the root <code>File</code> to delete
* @param deleteRoot whether or not to delete the root itself or just the content of the root.
* @return <code>true</code> if the <code>File</code> was deleted,
* otherwise <code>false</code>
*/
public static boolean deleteRecursively(File root, boolean deleteRoot) {
if (root != null && root.exists()) {
if (root.isDirectory()) {
File[] children = root.listFiles();
if (children != null) {
for (File aChildren : children) {
deleteRecursively(aChildren, true);
public static void deleteSubDirectories(Path... paths) throws IOException {
for (Path path : paths) {
try (DirectoryStream<Path> stream = Files.newDirectoryStream(path)) {
for (Path subPath : stream) {
if (Files.isDirectory(subPath)) {
IOUtils.rm(subPath);
}
}
}

if (deleteRoot) {
return root.delete();
} else {
return true;
}
}
return false;
}

/**
* Ensure that any writes to the given file is written to the storage device that contains it.
* @param fileToSync the file to fsync
* @param isDir if true, the given file is a directory (we open for read and ignore IOExceptions,
* because not all file systems and operating systems allow to fsync on a directory)
*/
public static void syncFile(File fileToSync, boolean isDir) throws IOException {
IOUtils.fsync(fileToSync.toPath(), isDir);
}

/**
* Check that a directory exists, is a directory and is readable
Expand All @@ -181,11 +131,19 @@ public static boolean isAccessibleDirectory(File directory, ESLogger logger) {

private FileSystemUtils() {}

public static void tryDeleteFile(File file) {
try {
file.delete();
} catch (SecurityException e1) {
// ignore
/**
* Temporary solution until LUCENE-6051 is fixed
* @see org.apache.lucene.util.IOUtils#deleteFilesIgnoringExceptions(java.nio.file.Path...)
*/
public static void deleteFilesIgnoringExceptions(Path... files) {
for (Path name : files) {
if (name != null) {
try {
Files.delete(name);
} catch (Throwable ignored) {
// ignore
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.carrotsearch.hppc.ObjectFloatOpenHashMap;
import com.carrotsearch.hppc.ObjectOpenHashSet;
import com.carrotsearch.hppc.cursors.ObjectCursor;
import org.apache.lucene.util.IOUtils;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.action.FailedNodeException;
import org.elasticsearch.cluster.*;
Expand Down Expand Up @@ -197,7 +198,11 @@ public Class<? extends Module> suggestIndexGateway() {

@Override
public void reset() throws Exception {
FileSystemUtils.deleteRecursively(nodeEnv.nodeDataLocations());
try {
IOUtils.rm(FileSystemUtils.toPaths(nodeEnv.nodeDataLocations()));
} catch (Exception ex) {
logger.debug("failed to delete shard locations", ex);
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import org.apache.lucene.util.IOUtils;
import org.elasticsearch.ElasticsearchIllegalArgumentException;
import org.elasticsearch.ElasticsearchIllegalStateException;
import org.elasticsearch.Version;
Expand Down Expand Up @@ -49,6 +50,7 @@
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.nio.file.Files;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -243,7 +245,11 @@ public void clusterChanged(ClusterChangedEvent event) {
if (!newMetaData.hasIndex(current.index())) {
logger.debug("[{}] deleting index that is no longer part of the metadata (indices: [{}])", current.index(), newMetaData.indices().keys());
if (nodeEnv.hasNodeFile()) {
FileSystemUtils.deleteRecursively(nodeEnv.indexLocations(new Index(current.index())));
try {
IOUtils.rm(FileSystemUtils.toPaths(nodeEnv.indexLocations(new Index(current.index()))));
} catch (Exception ex) {
logger.debug("[{}] failed to delete index", ex, current.index());
}
}
try {
nodeIndexDeletedAction.nodeIndexStoreDeleted(event.state(), current.index(), event.state().nodes().localNodeId());
Expand Down Expand Up @@ -280,7 +286,11 @@ public void clusterChanged(ClusterChangedEvent event) {
if (indexMetaData != null) {
if (danglingTimeout.millis() == 0) {
logger.info("[{}] dangling index, exists on local file system, but not in cluster metadata, timeout set to 0, deleting now", indexName);
FileSystemUtils.deleteRecursively(nodeEnv.indexLocations(new Index(indexName)));
try {
IOUtils.rm(FileSystemUtils.toPaths(nodeEnv.indexLocations(new Index(indexName))));
} catch (Exception ex) {
logger.debug("[{}] failed to delete dangling index", ex, indexName);
}
} else {
logger.info("[{}] dangling index, exists on local file system, but not in cluster metadata, scheduling to delete in [{}], auto import to cluster state [{}]", indexName, danglingTimeout, autoImportDangled);
danglingIndices.put(indexName, new DanglingIndex(indexName, threadPool.schedule(danglingTimeout, ThreadPool.Names.SAME, new RemoveDanglingIndex(indexName))));
Expand Down Expand Up @@ -517,7 +527,11 @@ private void pre019Upgrade() throws Exception {
if (!name.startsWith("metadata-")) {
continue;
}
stateFile.delete();
try {
Files.delete(stateFile.toPath());
} catch (Exception ex) {
logger.debug("failed to delete file " + stateFile, ex);
}
}
}

Expand Down Expand Up @@ -576,7 +590,11 @@ public void run() {
return;
}
logger.warn("[{}] deleting dangling index", index);
FileSystemUtils.deleteRecursively(nodeEnv.indexLocations(new Index(index)));
try {
IOUtils.rm(FileSystemUtils.toPaths(nodeEnv.indexLocations(new Index(index))));
} catch (Exception ex) {
logger.debug("failed to delete dangling index", ex);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,9 @@ private void cleanupOldFiles(String prefix, String fileName, File[] locations) t
if (file.getName().equals(fileName)) {
continue;
}
Files.delete(file.toPath());
if (Files.exists(file.toPath())) {
Files.delete(file.toPath());
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.elasticsearch.index.shard.ShardId;

import java.io.*;
import java.nio.file.Files;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -317,7 +318,12 @@ private void pre019Upgrade() throws Exception {
if (!name.startsWith("shards-")) {
continue;
}
stateFile.delete();
try {
Files.delete(stateFile.toPath());
} catch (Exception ex) {
logger.debug("Failed to delete state file {}", ex, stateFile);
}

}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.gateway.none;

import org.apache.lucene.util.IOUtils;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.cluster.*;
import org.elasticsearch.cluster.action.index.NodeIndexDeletedAction;
Expand Down Expand Up @@ -117,7 +118,11 @@ public void clusterChanged(ClusterChangedEvent event) {
if (!newMetaData.hasIndex(current.index())) {
logger.debug("[{}] deleting index that is no longer part of the metadata (indices: [{}])", current.index(), newMetaData.indices().keys());
if (nodeEnv.hasNodeFile()) {
FileSystemUtils.deleteRecursively(nodeEnv.indexLocations(new Index(current.index())));
try {
IOUtils.rm(FileSystemUtils.toPaths(nodeEnv.indexLocations(new Index(current.index()))));
} catch (Exception ex) {
logger.debug("failed to delete shard locations", ex);
}
}
try {
nodeIndexDeletedAction.nodeIndexStoreDeleted(event.state(), current.index(), event.state().nodes().localNodeId());
Expand Down
Loading