-
Notifications
You must be signed in to change notification settings - Fork 821
Fix HTTP 500 in filestore getMetadata racing with concurrent delete #4367
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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: [] | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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<FileStore.FileDetails> 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(); | ||||||||||||||||||||
|
Comment on lines
+264
to
265
|
||||||||||||||||||||
| entryMetadata.size = size; | |
| entryMetadata.timestamp = details.getTimeStamp(); | |
| final var timestamp = details.getTimeStamp(); | |
| if (timestamp == null) { | |
| // File was deleted concurrently between reading its size and timestamp. | |
| return null; | |
| } | |
| entryMetadata.size = size; | |
| entryMetadata.timestamp = timestamp; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changelog entries in this directory typically include a link (PR and/or JIRA) under
links:rather than an empty list. Please add an appropriate link entry so the change is traceable from the release notes.