Copilot commented on code in PR #4367: URL: https://github.com/apache/solr/pull/4367#discussion_r3144128017
########## 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: [] Review Comment: 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. ```suggestion links: - https://github.com/apache/solr/pull/4367 ``` ########## solr/core/src/java/org/apache/solr/filestore/ClusterFileStore.java: ########## @@ -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(); Review Comment: `convertToResponse` treats `size < 0` as a concurrent-delete race, but `getTimeStamp()` can also now return `null` on the same race if the file is deleted after `size()` succeeds. To keep the response consistent with NOFILE semantics, consider treating a `null` timestamp as a vanished entry as well (return `null` from `convertToResponse`). ```suggestion 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; ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
