Skip to content

Commit 95171e2

Browse files
committed
[CORE] Cut over to Path API for file deletion
Today we use the File API for file deletion as well as recursive directory deletions. This API returns a boolean if operations are successful while hiding the actual reason why they failed. The Path API throws and actual exception that might provide better insights and debug information. Closes #8366
1 parent 9ebce34 commit 95171e2

34 files changed

+244
-153
lines changed
+2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
11
@defaultMessage Convert to URI
22
java.net.URL#getPath()
33
java.net.URL#getFile()
4+
5+
java.io.File#delete() @ use Files.delete for real exception, IOUtils.deleteFilesIgnoringExceptions if you dont care

src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ interface BlobNameFilter {
5151
*/
5252
OutputStream createOutput(String blobName) throws IOException;
5353

54-
boolean deleteBlob(String blobName) throws IOException;
54+
void deleteBlob(String blobName) throws IOException;
5555

5656
void deleteBlobsByPrefix(String blobNamePrefix) throws IOException;
5757

src/main/java/org/elasticsearch/common/blobstore/BlobStore.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,16 @@
1818
*/
1919
package org.elasticsearch.common.blobstore;
2020

21+
import java.io.IOException;
22+
2123
/**
2224
*
2325
*/
2426
public interface BlobStore {
2527

2628
BlobContainer blobContainer(BlobPath path);
2729

28-
void delete(BlobPath path);
30+
void delete(BlobPath path) throws IOException;
2931

3032
void close();
3133
}

src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java

+8-2
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
import org.elasticsearch.common.io.FileSystemUtils;
3131

3232
import java.io.*;
33+
import java.nio.file.Files;
34+
import java.nio.file.Path;
3335

3436
/**
3537
*
@@ -65,8 +67,12 @@ public ImmutableMap<String, BlobMetaData> listBlobs() throws IOException {
6567
return builder.immutableMap();
6668
}
6769

68-
public boolean deleteBlob(String blobName) throws IOException {
69-
return new File(path, blobName).delete();
70+
@Override
71+
public void deleteBlob(String blobName) throws IOException {
72+
Path blobPath = new File(path, blobName).toPath();
73+
if (Files.exists(blobPath)) {
74+
Files.delete(blobPath);
75+
}
7076
}
7177

7278
@Override

src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobStore.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
package org.elasticsearch.common.blobstore.fs;
2121

22+
import org.apache.lucene.util.IOUtils;
2223
import org.elasticsearch.common.blobstore.BlobContainer;
2324
import org.elasticsearch.common.blobstore.BlobPath;
2425
import org.elasticsearch.common.blobstore.BlobStore;
@@ -30,6 +31,7 @@
3031
import org.elasticsearch.common.unit.ByteSizeValue;
3132

3233
import java.io.File;
34+
import java.io.IOException;
3335

3436
/**
3537
*
@@ -74,8 +76,8 @@ public BlobContainer blobContainer(BlobPath path) {
7476
}
7577

7678
@Override
77-
public void delete(BlobPath path) {
78-
FileSystemUtils.deleteRecursively(buildPath(path));
79+
public void delete(BlobPath path) throws IOException {
80+
IOUtils.rm(buildPath(path).toPath());
7981
}
8082

8183
@Override

src/main/java/org/elasticsearch/common/blobstore/url/URLBlobContainer.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public ImmutableMap<String, BlobMetaData> listBlobs() throws IOException {
7373
* This operation is not supported by URLBlobContainer
7474
*/
7575
@Override
76-
public boolean deleteBlob(String blobName) throws IOException {
76+
public void deleteBlob(String blobName) throws IOException {
7777
throw new UnsupportedOperationException("URL repository is read only");
7878
}
7979

src/main/java/org/elasticsearch/common/http/client/HttpDownloadHelper.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.apache.lucene.util.IOUtils;
2323
import org.elasticsearch.ElasticsearchTimeoutException;
2424
import org.elasticsearch.common.Nullable;
25+
import org.elasticsearch.common.io.FileSystemUtils;
2526
import org.elasticsearch.common.unit.TimeValue;
2627

2728
import java.io.*;
@@ -346,7 +347,7 @@ private boolean downloadFile() throws FileNotFoundException, IOException {
346347
// Try to delete the garbage we'd otherwise leave
347348
// behind.
348349
IOUtils.closeWhileHandlingException(os, is);
349-
dest.delete();
350+
FileSystemUtils.deleteFilesIgnoringExceptions(dest.toPath());
350351
} else {
351352
IOUtils.close(os, is);
352353
}
@@ -385,7 +386,7 @@ void closeStreams() throws IOException {
385386
} else {
386387
IOUtils.closeWhileHandlingException(is, os);
387388
if (dest != null && dest.exists()) {
388-
dest.delete();
389+
FileSystemUtils.deleteFilesIgnoringExceptions(dest.toPath());
389390
}
390391
}
391392
}

src/main/java/org/elasticsearch/common/io/FileSystemUtils.java

+29-71
Original file line numberDiff line numberDiff line change
@@ -78,84 +78,34 @@ public static boolean exists(File... files) {
7878
}
7979

8080
/**
81-
* Deletes the given files recursively. if <tt>deleteRoots</tt> is set to <code>true</code>
82-
* the given root files will be deleted as well. Otherwise only their content is deleted.
81+
* Returns an array of {@link Path} build from the correspondent element
82+
* in the input array using {@link java.io.File#toPath()}
83+
* @param files the files to get paths for
8384
*/
84-
public static boolean deleteRecursively(File[] roots, boolean deleteRoots) {
85-
86-
boolean deleted = true;
87-
for (File root : roots) {
88-
deleted &= deleteRecursively(root, deleteRoots);
85+
public static Path[] toPaths(File... files) {
86+
Path[] paths = new Path[files.length];
87+
for (int i = 0; i < files.length; i++) {
88+
paths[i] = files[i].toPath();
8989
}
90-
return deleted;
90+
return paths;
9191
}
9292

9393
/**
94-
* Deletes all subdirectories of the given roots recursively.
94+
* Deletes all subdirectories in the given path recursively
95+
* @throws java.lang.IllegalArgumentException if the given path is not a directory
9596
*/
96-
public static boolean deleteSubDirectories(File[] roots) {
97-
98-
boolean deleted = true;
99-
for (File root : roots) {
100-
if (root.isDirectory()) {
101-
File[] files = root.listFiles(new FileFilter() {
102-
@Override
103-
public boolean accept(File pathname) {
104-
return pathname.isDirectory();
105-
}
106-
});
107-
deleted &= deleteRecursively(files, true);
108-
}
109-
110-
}
111-
return deleted;
112-
}
113-
114-
/**
115-
* Deletes the given files recursively including the given roots.
116-
*/
117-
public static boolean deleteRecursively(File... roots) {
118-
return deleteRecursively(roots, true);
119-
}
120-
121-
/**
122-
* Delete the supplied {@link java.io.File} - for directories,
123-
* recursively delete any nested directories or files as well.
124-
*
125-
* @param root the root <code>File</code> to delete
126-
* @param deleteRoot whether or not to delete the root itself or just the content of the root.
127-
* @return <code>true</code> if the <code>File</code> was deleted,
128-
* otherwise <code>false</code>
129-
*/
130-
public static boolean deleteRecursively(File root, boolean deleteRoot) {
131-
if (root != null && root.exists()) {
132-
if (root.isDirectory()) {
133-
File[] children = root.listFiles();
134-
if (children != null) {
135-
for (File aChildren : children) {
136-
deleteRecursively(aChildren, true);
97+
public static void deleteSubDirectories(Path... paths) throws IOException {
98+
for (Path path : paths) {
99+
try (DirectoryStream<Path> stream = Files.newDirectoryStream(path)) {
100+
for (Path subPath : stream) {
101+
if (Files.isDirectory(subPath)) {
102+
IOUtils.rm(subPath);
137103
}
138104
}
139105
}
140-
141-
if (deleteRoot) {
142-
return root.delete();
143-
} else {
144-
return true;
145-
}
146106
}
147-
return false;
148107
}
149108

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

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

182132
private FileSystemUtils() {}
183133

184-
public static void tryDeleteFile(File file) {
185-
try {
186-
file.delete();
187-
} catch (SecurityException e1) {
188-
// ignore
134+
/**
135+
* Temporary solution until LUCENE-6051 is fixed
136+
* @see org.apache.lucene.util.IOUtils#deleteFilesIgnoringExceptions(java.nio.file.Path...)
137+
*/
138+
public static void deleteFilesIgnoringExceptions(Path... files) {
139+
for (Path name : files) {
140+
if (name != null) {
141+
try {
142+
Files.delete(name);
143+
} catch (Throwable ignored) {
144+
// ignore
145+
}
146+
}
189147
}
190148
}
191149

src/main/java/org/elasticsearch/gateway/local/LocalGateway.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.carrotsearch.hppc.ObjectFloatOpenHashMap;
2323
import com.carrotsearch.hppc.ObjectOpenHashSet;
2424
import com.carrotsearch.hppc.cursors.ObjectCursor;
25+
import org.apache.lucene.util.IOUtils;
2526
import org.elasticsearch.ElasticsearchException;
2627
import org.elasticsearch.action.FailedNodeException;
2728
import org.elasticsearch.cluster.*;
@@ -197,7 +198,11 @@ public Class<? extends Module> suggestIndexGateway() {
197198

198199
@Override
199200
public void reset() throws Exception {
200-
FileSystemUtils.deleteRecursively(nodeEnv.nodeDataLocations());
201+
try {
202+
IOUtils.rm(FileSystemUtils.toPaths(nodeEnv.nodeDataLocations()));
203+
} catch (Exception ex) {
204+
logger.debug("failed to delete shard locations", ex);
205+
}
201206
}
202207

203208
@Override

src/main/java/org/elasticsearch/gateway/local/state/meta/LocalGatewayMetaState.java

+22-4
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import com.google.common.collect.Lists;
2323
import com.google.common.collect.Maps;
24+
import org.apache.lucene.util.IOUtils;
2425
import org.elasticsearch.ElasticsearchIllegalArgumentException;
2526
import org.elasticsearch.ElasticsearchIllegalStateException;
2627
import org.elasticsearch.Version;
@@ -49,6 +50,7 @@
4950
import java.io.File;
5051
import java.io.FileInputStream;
5152
import java.io.IOException;
53+
import java.nio.file.Files;
5254
import java.util.List;
5355
import java.util.Map;
5456
import java.util.Set;
@@ -243,7 +245,11 @@ public void clusterChanged(ClusterChangedEvent event) {
243245
if (!newMetaData.hasIndex(current.index())) {
244246
logger.debug("[{}] deleting index that is no longer part of the metadata (indices: [{}])", current.index(), newMetaData.indices().keys());
245247
if (nodeEnv.hasNodeFile()) {
246-
FileSystemUtils.deleteRecursively(nodeEnv.indexLocations(new Index(current.index())));
248+
try {
249+
IOUtils.rm(FileSystemUtils.toPaths(nodeEnv.indexLocations(new Index(current.index()))));
250+
} catch (Exception ex) {
251+
logger.debug("[{}] failed to delete index", ex, current.index());
252+
}
247253
}
248254
try {
249255
nodeIndexDeletedAction.nodeIndexStoreDeleted(event.state(), current.index(), event.state().nodes().localNodeId());
@@ -280,7 +286,11 @@ public void clusterChanged(ClusterChangedEvent event) {
280286
if (indexMetaData != null) {
281287
if (danglingTimeout.millis() == 0) {
282288
logger.info("[{}] dangling index, exists on local file system, but not in cluster metadata, timeout set to 0, deleting now", indexName);
283-
FileSystemUtils.deleteRecursively(nodeEnv.indexLocations(new Index(indexName)));
289+
try {
290+
IOUtils.rm(FileSystemUtils.toPaths(nodeEnv.indexLocations(new Index(indexName))));
291+
} catch (Exception ex) {
292+
logger.debug("[{}] failed to delete dangling index", ex, indexName);
293+
}
284294
} else {
285295
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);
286296
danglingIndices.put(indexName, new DanglingIndex(indexName, threadPool.schedule(danglingTimeout, ThreadPool.Names.SAME, new RemoveDanglingIndex(indexName))));
@@ -517,7 +527,11 @@ private void pre019Upgrade() throws Exception {
517527
if (!name.startsWith("metadata-")) {
518528
continue;
519529
}
520-
stateFile.delete();
530+
try {
531+
Files.delete(stateFile.toPath());
532+
} catch (Exception ex) {
533+
logger.debug("failed to delete file " + stateFile, ex);
534+
}
521535
}
522536
}
523537

@@ -576,7 +590,11 @@ public void run() {
576590
return;
577591
}
578592
logger.warn("[{}] deleting dangling index", index);
579-
FileSystemUtils.deleteRecursively(nodeEnv.indexLocations(new Index(index)));
593+
try {
594+
IOUtils.rm(FileSystemUtils.toPaths(nodeEnv.indexLocations(new Index(index))));
595+
} catch (Exception ex) {
596+
logger.debug("failed to delete dangling index", ex);
597+
}
580598
}
581599
}
582600
}

src/main/java/org/elasticsearch/gateway/local/state/meta/MetaDataStateFormat.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,9 @@ private void cleanupOldFiles(String prefix, String fileName, File[] locations) t
208208
if (file.getName().equals(fileName)) {
209209
continue;
210210
}
211-
Files.delete(file.toPath());
211+
if (Files.exists(file.toPath())) {
212+
Files.delete(file.toPath());
213+
}
212214
}
213215
}
214216
}

src/main/java/org/elasticsearch/gateway/local/state/shards/LocalGatewayShardsState.java

+7-1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.elasticsearch.index.shard.ShardId;
3838

3939
import java.io.*;
40+
import java.nio.file.Files;
4041
import java.util.Iterator;
4142
import java.util.Map;
4243
import java.util.Set;
@@ -317,7 +318,12 @@ private void pre019Upgrade() throws Exception {
317318
if (!name.startsWith("shards-")) {
318319
continue;
319320
}
320-
stateFile.delete();
321+
try {
322+
Files.delete(stateFile.toPath());
323+
} catch (Exception ex) {
324+
logger.debug("Failed to delete state file {}", ex, stateFile);
325+
}
326+
321327
}
322328
}
323329

src/main/java/org/elasticsearch/gateway/none/NoneGateway.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
package org.elasticsearch.gateway.none;
2121

22+
import org.apache.lucene.util.IOUtils;
2223
import org.elasticsearch.ElasticsearchException;
2324
import org.elasticsearch.cluster.*;
2425
import org.elasticsearch.cluster.action.index.NodeIndexDeletedAction;
@@ -117,7 +118,11 @@ public void clusterChanged(ClusterChangedEvent event) {
117118
if (!newMetaData.hasIndex(current.index())) {
118119
logger.debug("[{}] deleting index that is no longer part of the metadata (indices: [{}])", current.index(), newMetaData.indices().keys());
119120
if (nodeEnv.hasNodeFile()) {
120-
FileSystemUtils.deleteRecursively(nodeEnv.indexLocations(new Index(current.index())));
121+
try {
122+
IOUtils.rm(FileSystemUtils.toPaths(nodeEnv.indexLocations(new Index(current.index()))));
123+
} catch (Exception ex) {
124+
logger.debug("failed to delete shard locations", ex);
125+
}
121126
}
122127
try {
123128
nodeIndexDeletedAction.nodeIndexStoreDeleted(event.state(), current.index(), event.state().nodes().localNodeId());

0 commit comments

Comments
 (0)