Skip to content
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.

Sign up for GitHub

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

SOLR-17351: Decompose filestore "get file" API #3047

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

SOLR-17351: Decompose filestore "get file" API #3047

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
070cd64
Tweak path on /api/cluster/files APIs
gerlowskija Jan 4, 2025
11253ac
Create API for file/data metadata fetching
gerlowskija Jan 5, 2025
e79ae04
Create API for raw file fetching
gerlowskija Jan 8, 2025
909e7a2
Create new filestore getFrom/sync API
gerlowskija Jan 8, 2025
cbbd5dd
Fix test failures
gerlowskija Jan 9, 2025
aa30575
Allow generation of 'InputStream' responses
gerlowskija Jan 9, 2025
4d249d6
WIP: Initial attempt to switch over DistribFileStore
gerlowskija Jan 10, 2025
2ac6dcb
Temporary stash to investigate param screening behavior
gerlowskija Jan 10, 2025
0ed3367
Merge branch 'main' into SOLR-17351-break-up-getfile-api-main
gerlowskija Jan 20, 2025
5f77816
Generate 'setQueryParams' method
gerlowskija Jan 20, 2025
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
48 changes: 47 additions & 1 deletion solr/api/src/java/org/apache/solr/client/api/endpoint/ClusterFileStoreApis.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,27 @@
package org.apache.solr.client.api.endpoint;

import static org.apache.solr.client.api.util.Constants.GENERIC_ENTITY_PROPERTY;
import static org.apache.solr.client.api.util.Constants.RAW_OUTPUT_PROPERTY;

import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.Parameter;
import io.swagger.v3.oas.annotations.extensions.Extension;
import io.swagger.v3.oas.annotations.extensions.ExtensionProperty;
import io.swagger.v3.oas.annotations.parameters.RequestBody;
import jakarta.ws.rs.DELETE;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.POST;
import jakarta.ws.rs.PUT;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import jakarta.ws.rs.QueryParam;
import java.io.InputStream;
import java.util.List;
import org.apache.solr.client.api.model.FileStoreDirectoryListingResponse;
import org.apache.solr.client.api.model.SolrJerseyResponse;
import org.apache.solr.client.api.model.UploadToFileStoreResponse;

@Path("/cluster")
@Path("/cluster/filestore")
public interface ClusterFileStoreApis {
// TODO Better understand the purpose of the 'sig' parameter and improve docs here.
@PUT
Expand All @@ -56,6 +60,29 @@ UploadToFileStoreResponse uploadFile(
})
InputStream requestBody);

@GET
@Operation(
summary = "Retrieve metadata about a file or directory in the filestore.",
tags = {"file-store"})
@Path("/metadata{path:.+}")
FileStoreDirectoryListingResponse getMetadata(
@Parameter(description = "Path to a file or directory within the filestore")
@PathParam("path")
String path);

@GET
@Operation(
summary = "Retrieve raw contents of a file in the filestore.",
tags = {"file-store"},
extensions = {
@Extension(properties = {@ExtensionProperty(name = RAW_OUTPUT_PROPERTY, value = "true")})
})
@Path("/files{filePath:.+}")
SolrJerseyResponse getFile(
@Parameter(description = "Path to a file or directory within the filestore")
@PathParam("filePath")
String path);

@DELETE
@Operation(
summary = "Delete a file or directory from the filestore.",
Expand All @@ -70,4 +97,23 @@ SolrJerseyResponse deleteFile(
"Indicates whether the deletion should only be done on the receiving node. For internal use only")
@QueryParam("localDelete")
Boolean localDelete);

@POST
@Operation(
summary =
"Pushes a file to other nodes, or pulls a file from other nodes in the Solr cluster.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the fact that one thing does a push or pull suggest a issue to fix? https://en.wikipedia.org/wiki/List_of_Doctor_Dolittle_characters#Pushmi-Pullyu

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Syncs a file via either pushing or pulling across the nodes in the Solr cluster." ???

tags = {"file-store"})
@Path("/commands{path:.+}")
SolrJerseyResponse executeFileStoreCommand(
@Parameter(description = "Path to a file or directory within the filestore")
@PathParam("path")
String path,
@Parameter(description = "An optional Solr node name to fetch the file from")
@QueryParam("getFrom")
String getFrom,
@Parameter(
description =
"If true, triggers syncing for this file across all nodes in the filestore")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe it's own end point? Or, do we somehow eliminate the need for this?

@QueryParam("sync")
Boolean sync);
}
149 changes: 149 additions & 0 deletions solr/core/src/java/org/apache/solr/filestore/ClusterFileStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,33 @@
package org.apache.solr.filestore;

import static java.nio.charset.StandardCharsets.UTF_8;
import static org.apache.solr.handler.admin.api.ReplicationAPIBase.FILE_STREAM;
import static org.apache.solr.response.RawResponseWriter.CONTENT;

import jakarta.inject.Inject;
import java.io.IOException;
import java.io.InputStream;
import java.lang.invoke.MethodHandles;
import java.nio.ByteBuffer;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import org.apache.commons.codec.digest.DigestUtils;
import org.apache.solr.api.JerseyResource;
import org.apache.solr.client.api.endpoint.ClusterFileStoreApis;
import org.apache.solr.client.api.model.FileStoreDirectoryListingResponse;
import org.apache.solr.client.api.model.FileStoreEntryMetadata;
import org.apache.solr.client.api.model.SolrJerseyResponse;
import org.apache.solr.client.api.model.UploadToFileStoreResponse;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.StrUtils;
import org.apache.solr.core.CoreContainer;
import org.apache.solr.core.SolrCore;
import org.apache.solr.jersey.PermissionName;
import org.apache.solr.pkg.PackageAPI;
import org.apache.solr.request.SolrQueryRequest;
Expand Down Expand Up @@ -141,6 +151,103 @@ public UploadToFileStoreResponse uploadFile(
return response;
}

@Override
@PermissionName(PermissionNameProvider.Name.FILESTORE_READ_PERM)
public SolrJerseyResponse getFile(String path) {
final var response = instantiateJerseyResponse(SolrJerseyResponse.class);
attachFileToResponse(path, fileStore, req, rsp);
return response;
}

@Override
@PermissionName(PermissionNameProvider.Name.FILESTORE_READ_PERM)
public FileStoreDirectoryListingResponse getMetadata(String path) {
if (path == null) {
path = "";
}
FileStore.FileType type = fileStore.getType(path, false);
return getMetadata(type, path, fileStore);
}

public static void attachFileToResponse(
String path, FileStore fileStore, SolrQueryRequest req, SolrQueryResponse rsp) {
ModifiableSolrParams solrParams = new ModifiableSolrParams();
solrParams.add(CommonParams.WT, FILE_STREAM);
req.setParams(SolrParams.wrapDefaults(solrParams, req.getParams()));
rsp.add(
CONTENT,
(SolrCore.RawWriter)
os ->
fileStore.get(
path,
it -> {
try {
InputStream inputStream = it.getInputStream();
if (inputStream != null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could this ever be null? wouldn't you have an exception instead?

inputStream.transferTo(os);
}
} catch (IOException e) {
throw new SolrException(
SolrException.ErrorCode.SERVER_ERROR, "Error reading file " + path);
}
},
false));
}

@SuppressWarnings("fallthrough")
public static FileStoreDirectoryListingResponse getMetadata(
FileStore.FileType type, String path, FileStore fileStore) {
final var dirListingResponse = new FileStoreDirectoryListingResponse();
if (path == null) {
path = "";
}

switch (type) {
case NOFILE:
dirListingResponse.files = Collections.singletonMap(path, null);
break;
case METADATA:
case FILE:
int idx = path.lastIndexOf('/');
String fileName = path.substring(idx + 1);
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)));
break;
case DIRECTORY:
final var directoryContents =
fileStore.list(path, null).stream()
.map(details -> convertToResponse(details))
.collect(Collectors.toList());
dirListingResponse.files = Collections.singletonMap(path, directoryContents);
break;
}

return dirListingResponse;
}

// TODO Modify the filestore implementation itself to return this object, so conversion isn't
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think i also ran into this in the schema designer

// needed.
private static FileStoreEntryMetadata convertToResponse(FileStore.FileDetails details) {
final var entryMetadata = new FileStoreEntryMetadata();

entryMetadata.name = details.getSimpleName();
if (details.isDir()) {
entryMetadata.dir = true;
return entryMetadata;
}

entryMetadata.size = details.size();
entryMetadata.timestamp = details.getTimeStamp();
if (details.getMetaData() != null) {
details.getMetaData().toMap(entryMetadata.unknownProperties());
}

return entryMetadata;
}

private void doLocalDelete(String filePath) {
fileStore.deleteLocal(filePath);
}
Expand Down Expand Up @@ -194,6 +301,48 @@ public SolrJerseyResponse deleteFile(String filePath, Boolean localDelete) {
return response;
}

@Override
@PermissionName(PermissionNameProvider.Name.FILESTORE_WRITE_PERM)
public SolrJerseyResponse executeFileStoreCommand(String path, String getFrom, Boolean sync) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aren't we trying to get rid of the "command" pattern in our new APIs?

final var response = instantiateJerseyResponse(SolrJerseyResponse.class);

if (Boolean.TRUE.equals(sync)) {
syncToAllNodes(fileStore, path);
} else if (getFrom != null) {
if (path == null) {
path = "";
}
pullFileFromNode(coreContainer, fileStore, path, getFrom);
}

return response;
}

public static void syncToAllNodes(FileStore fileStore, String path) {
try {
fileStore.syncToAllNodes(path);
} catch (IOException e) {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Error getting file ", e);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't know if we can blame the user

}
}

public static void pullFileFromNode(
CoreContainer coreContainer, FileStore fileStore, String path, String getFrom) {
coreContainer
.getUpdateShardHandler()
.getUpdateExecutor()
.submit(
() -> {
log.debug("Downloading file {}", path);
try {
fileStore.fetch(path, getFrom);
} catch (Exception e) {
log.error("Failed to download file: {}", path, e);
}
log.info("downloaded file: {}", path);
});
}

private List<String> readSignatures(List<String> signatures, byte[] buf)
throws SolrException, IOException {
if (signatures == null || signatures.isEmpty()) return null;
Expand Down
Loading
Loading