diff --git a/changelog/unreleased/PR#4367-filestore-getmetadata-delete-race.yml b/changelog/unreleased/PR#4367-filestore-getmetadata-delete-race.yml new file mode 100644 index 000000000000..0d310d48b920 --- /dev/null +++ b/changelog/unreleased/PR#4367-filestore-getmetadata-delete-race.yml @@ -0,0 +1,6 @@ +# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc +title: Filestore metadata API no longer returns HTTP 500 when a file is deleted concurrently with a metadata read; the response now matches the not-found case (null entry). +type: fixed # added, changed, fixed, deprecated, removed, dependency_update, security, other +authors: + - name: Eric Pugh +links: [] diff --git a/solr/core/src/java/org/apache/solr/filestore/ClusterFileStore.java b/solr/core/src/java/org/apache/solr/filestore/ClusterFileStore.java index c405fcbcfe95..5049d3022f0c 100644 --- a/solr/core/src/java/org/apache/solr/filestore/ClusterFileStore.java +++ b/solr/core/src/java/org/apache/solr/filestore/ClusterFileStore.java @@ -31,6 +31,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.stream.Collectors; import org.apache.commons.codec.digest.DigestUtils; import org.apache.solr.api.JerseyResource; @@ -228,13 +229,14 @@ public static FileStoreDirectoryListingResponse getMetadata( String parentPath = path.substring(0, path.lastIndexOf('/')); List l = fileStore.list(parentPath, s -> s.equals(fileName)); - dirListingResponse.files = - Collections.singletonMap(path, l.isEmpty() ? null : convertToResponse(l.get(0))); + FileStoreEntryMetadata entry = l.isEmpty() ? null : convertToResponse(l.get(0)); + dirListingResponse.files = Collections.singletonMap(path, entry); break; case DIRECTORY: final var directoryContents = fileStore.list(path, null).stream() .map(details -> convertToResponse(details)) + .filter(Objects::nonNull) .collect(Collectors.toList()); dirListingResponse.files = Map.of(path, directoryContents); break; @@ -254,7 +256,12 @@ private static FileStoreEntryMetadata convertToResponse(FileStore.FileDetails de return entryMetadata; } - entryMetadata.size = details.size(); + long size = details.size(); + if (size < 0) { + // File was deleted concurrently between listing and reading its attributes. + return null; + } + entryMetadata.size = size; entryMetadata.timestamp = details.getTimeStamp(); if (details.getMetaData() != null) { details.getMetaData().toMap(entryMetadata.unknownProperties()); diff --git a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java index 398075663a4c..7fffd9ebf08a 100644 --- a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java +++ b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java @@ -30,6 +30,7 @@ import java.nio.channels.SeekableByteChannel; import java.nio.file.FileSystems; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.StandardOpenOption; import java.util.ArrayList; @@ -288,6 +289,9 @@ public MetaData getMetaData() { public Date getTimeStamp() { try { return new Date(Files.getLastModifiedTime(realPath()).toMillis()); + } catch (NoSuchFileException e) { + // File was deleted concurrently between listing and reading its attributes. + return null; } catch (IOException e) { throw new SolrException( SERVER_ERROR, "Failed to retrieve the last modified time for: " + realPath(), e); @@ -303,6 +307,9 @@ public boolean isDir() { public long size() { try { return Files.size(realPath()); + } catch (NoSuchFileException e) { + // File was deleted concurrently between listing and reading its attributes. + return -1; } catch (IOException e) { throw new SolrException( SERVER_ERROR, "Failed to retrieve the file size for: " + realPath(), e);