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

Jump to bottom

SOLR-15118: Convert /v2/collections APIs to annotations #2281

Merged
merged 13 commits into from Feb 8, 2021
Merged

SOLR-15118: Convert /v2/collections APIs to annotations #2281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
12586ee
SOLR-15118: Convert /v2/collections APIs to annotations
gerlowskija Jan 28, 2021
c613a1f
Use constants for param names where possible
gerlowskija Feb 1, 2021
472b7a4
Fix precommit, rd 1
gerlowskija Feb 1, 2021
f62f5b8
Merge branch 'master' into SOLR_15118_annotation_based_collections_apis
gerlowskija Feb 4, 2021
7b06110
Add new incremental backup/restore params to v2 POJOs
gerlowskija Feb 4, 2021
e4a0997
Remove unintentionally committed 'IML' file
gerlowskija Feb 4, 2021
f68afd1
Address review comments, rd 1
gerlowskija Feb 4, 2021
253676f
Address review feedback, rd 1 (cont'd)
gerlowskija Feb 4, 2021
6c616ad
Fix broken tests, pt 1
gerlowskija Feb 4, 2021
006b5c9
Remove V2RequestSupport impl from CollectionAdminRequest
gerlowskija Feb 7, 2021
5ffe72d
Fix precommit, rd 1
gerlowskija Feb 8, 2021
2f025b7
Fix precommit, rd 2
gerlowskija Feb 8, 2021
7ec7382
Fix test failures, rd 1
gerlowskija Feb 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion solr/core/src/java/org/apache/solr/core/CoreContainer.java
View file
Open in desktop
Expand Up @@ -743,7 +743,9 @@ public void load() {
createHandler(ZK_PATH, ZookeeperInfoHandler.class.getName(), ZookeeperInfoHandler.class);
createHandler(ZK_STATUS_PATH, ZookeeperStatusHandler.class.getName(), ZookeeperStatusHandler.class);
collectionsHandler = createHandler(COLLECTIONS_HANDLER_PATH, cfg.getCollectionsHandlerClass(), CollectionsHandler.class);
containerHandlers.getApiBag().registerObject(new CollectionsAPI(collectionsHandler));
final CollectionsAPI collectionsAPI = new CollectionsAPI(collectionsHandler);
containerHandlers.getApiBag().registerObject(collectionsAPI);
containerHandlers.getApiBag().registerObject(collectionsAPI.collectionsCommands);
configSetsHandler = createHandler(CONFIGSETS_HANDLER_PATH, cfg.getConfigSetsHandlerClass(), ConfigSetsHandler.class);
ClusterAPI clusterAPI = new ClusterAPI(collectionsHandler, configSetsHandler);
containerHandlers.getApiBag().registerObject(clusterAPI);
Expand Down
14 changes: 7 additions & 7 deletions solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
View file
Open in desktop
Expand Up @@ -23,9 +23,9 @@
import org.apache.solr.api.Command;
import org.apache.solr.api.EndPoint;
import org.apache.solr.api.PayloadObj;
import org.apache.solr.client.solrj.request.beans.ClusterPropInfo;
import org.apache.solr.client.solrj.request.beans.CreateConfigInfo;
import org.apache.solr.client.solrj.request.beans.RateLimiterMeta;
import org.apache.solr.client.solrj.request.beans.ClusterPropPayload;
import org.apache.solr.client.solrj.request.beans.CreateConfigPayload;
import org.apache.solr.client.solrj.request.beans.RateLimiterPayload;
import org.apache.solr.cloud.OverseerConfigSetMessageHandler;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.annotation.JsonProperty;
Expand Down Expand Up @@ -120,7 +120,7 @@ public class ConfigSetCommands {

@Command(name = "create")
@SuppressWarnings("unchecked")
public void create(PayloadObj<CreateConfigInfo> obj) throws Exception {
public void create(PayloadObj<CreateConfigPayload> obj) throws Exception {
Map<String, Object> mapVals = obj.get().toMap(new HashMap<>());
Map<String,Object> customProps = (Map<String, Object>) mapVals.remove("properties");
if(customProps!= null) {
Expand Down Expand Up @@ -223,7 +223,7 @@ public void removeRole(PayloadObj<RoleInfo> obj) throws Exception {

@Command(name = "set-obj-property")
@SuppressWarnings({"rawtypes", "unchecked"})
public void setObjProperty(PayloadObj<ClusterPropInfo> obj) {
public void setObjProperty(PayloadObj<ClusterPropPayload> obj) {
//Not using the object directly here because the API differentiate between {name:null} and {}
Map m = obj.getDataMap();
ClusterProperties clusterProperties = new ClusterProperties(getCoreContainer().getZkController().getZkClient());
Expand All @@ -242,8 +242,8 @@ public void setProperty(PayloadObj<Map<String,String>> obj) throws Exception {
}

@Command(name = "set-ratelimiter")
public void setRateLimiters(PayloadObj<RateLimiterMeta> payLoad) {
RateLimiterMeta rateLimiterConfig = payLoad.get();
public void setRateLimiters(PayloadObj<RateLimiterPayload> payLoad) {
RateLimiterPayload rateLimiterConfig = payLoad.get();
ClusterProperties clusterProperties = new ClusterProperties(getCoreContainer().getZkController().getZkClient());

try {
Expand Down
177 changes: 173 additions & 4 deletions solr/core/src/java/org/apache/solr/handler/CollectionsAPI.java
View file
Open in desktop
Expand Up @@ -17,16 +17,38 @@

package org.apache.solr.handler;

import org.apache.commons.collections4.CollectionUtils;
import org.apache.solr.api.Command;
import org.apache.solr.api.EndPoint;
import org.apache.solr.api.PayloadObj;
import org.apache.solr.client.solrj.request.beans.BackupCollectionPayload;
import org.apache.solr.client.solrj.request.beans.CreateAliasPayload;
import org.apache.solr.client.solrj.request.beans.CreatePayload;
import org.apache.solr.client.solrj.request.beans.DeleteAliasPayload;
import org.apache.solr.client.solrj.request.beans.RestoreCollectionPayload;
import org.apache.solr.client.solrj.request.beans.SetAliasPropertyPayload;
import org.apache.solr.client.solrj.request.beans.V2ApiConstants;
import org.apache.solr.common.cloud.ZkStateReader;
import org.apache.solr.common.params.CollectionAdminParams;
import org.apache.solr.common.params.CollectionParams.CollectionAction;
import org.apache.solr.handler.admin.CollectionsHandler;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.response.SolrQueryResponse;

import static org.apache.solr.client.solrj.SolrRequest.METHOD.DELETE;
import static org.apache.solr.client.solrj.SolrRequest.METHOD.GET;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

import static org.apache.solr.client.solrj.SolrRequest.METHOD.*;
import static org.apache.solr.client.solrj.request.beans.V2ApiConstants.ROUTER_KEY;
import static org.apache.solr.cloud.api.collections.RoutedAlias.CREATE_COLLECTION_PREFIX;
import static org.apache.solr.common.params.CollectionAdminParams.PROPERTY_PREFIX;
import static org.apache.solr.common.params.CollectionAdminParams.ROUTER_PREFIX;
import static org.apache.solr.common.params.CommonParams.ACTION;
import static org.apache.solr.common.params.CommonParams.NAME;
import static org.apache.solr.handler.ClusterAPI.wrapParams;
import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
import static org.apache.solr.security.PermissionNameProvider.Name.COLL_READ_PERM;

Expand All @@ -36,7 +58,16 @@
*/
public class CollectionsAPI {

private final CollectionsHandler collectionsHandler;
public static final String V2_CREATE_COLLECTION_CMD = "create";
public static final String V2_BACKUP_CMD = "backup-collection";
public static final String V2_RESTORE_CMD = "restore-collection";
public static final String V2_CREATE_ALIAS_CMD = "create-alias";
public static final String V2_SET_ALIAS_PROP_CMD = "set-alias-property";
public static final String V2_DELETE_ALIAS_CMD = "delete-alias";
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also add other collection commands here, such as: COLLECTIONPROP, CREATE / DELETESHARD, ADD / MOVE / DELETEREPLICA, MODIFYCOLLECTION, REINDEXCOLLECTION, RENAME, SPLITSHARD ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They should definitely exist somewhere, but they're not /v2/collections APIs, so I didn't do that in this PR. If you guys really want them, I'm not against adding those constants in this PR.

But even then they should probably live somewhere else, shouldn't they? (since this class is for /v2/collections APIs, and those commands are for other APIs? AFAIK this file is only for those APIs that specifically live under /v2/collections, right?)


private final CollectionsHandler collectionsHandler;

public final CollectionsCommands collectionsCommands = new CollectionsCommands();

public CollectionsAPI(CollectionsHandler collectionsHandler) {
this.collectionsHandler = collectionsHandler;
Expand All @@ -50,11 +81,149 @@ public void getCollections(SolrQueryRequest req, SolrQueryResponse rsp) throws E
CollectionsHandler.CollectionOperation.LIST_OP.execute(req, rsp, collectionsHandler);
}

@EndPoint(
path = {"/c", "/collections"},
method = POST,
permission = COLL_EDIT_PERM)
public class CollectionsCommands {

@Command(name = V2_BACKUP_CMD)
@SuppressWarnings("unchecked")
public void backupCollection(PayloadObj<BackupCollectionPayload> obj) throws Exception {
final Map<String, Object> v1Params = obj.get().toMap(new HashMap<>());
v1Params.put(ACTION, CollectionAction.BACKUP.toLower());

collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
}

@Command(name = V2_RESTORE_CMD)
@SuppressWarnings("unchecked")
public void restoreBackup(PayloadObj<RestoreCollectionPayload> obj) throws Exception {
final RestoreCollectionPayload v2Body = obj.get();
final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());

v1Params.put(ACTION, CollectionAction.RESTORE.toLower());
if (v2Body.createCollectionParams != null && !v2Body.createCollectionParams.isEmpty()) {
final Map<String, Object> createCollParams = (Map<String, Object>) v1Params.remove(V2ApiConstants.CREATE_COLLECTION_KEY);
convertV2CreateCollectionMapToV1ParamMap(createCollParams);
v1Params.putAll(createCollParams);
}

collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
}

@Command(name = V2_CREATE_ALIAS_CMD)
@SuppressWarnings("unchecked")
public void createAlias(PayloadObj<CreateAliasPayload> obj) throws Exception {
final CreateAliasPayload v2Body = obj.get();
final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());

v1Params.put(ACTION, CollectionAction.CREATEALIAS.toLower());
if (! CollectionUtils.isEmpty(v2Body.collections)) {
final String collectionsStr = String.join(",", v2Body.collections);
v1Params.remove(V2ApiConstants.COLLECTIONS);
v1Params.put(V2ApiConstants.COLLECTIONS, collectionsStr);
}
if (v2Body.router != null) {
Map<String, Object> routerProperties = (Map<String, Object>) v1Params.remove(V2ApiConstants.ROUTER_KEY);
flattenMapWithPrefix(routerProperties, v1Params, ROUTER_PREFIX);
}
if (v2Body.createCollectionParams != null && !v2Body.createCollectionParams.isEmpty()) {
final Map<String, Object> createCollectionMap = (Map<String, Object>) v1Params.remove(V2ApiConstants.CREATE_COLLECTION_KEY);
convertV2CreateCollectionMapToV1ParamMap(createCollectionMap);
flattenMapWithPrefix(createCollectionMap, v1Params, CREATE_COLLECTION_PREFIX);
}

collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
}

@Command(name= V2_SET_ALIAS_PROP_CMD)
@SuppressWarnings("unchecked")
public void setAliasProperty(PayloadObj<SetAliasPropertyPayload> obj) throws Exception {
final SetAliasPropertyPayload v2Body = obj.get();
final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());

v1Params.put(ACTION, CollectionAction.ALIASPROP.toLower());
// Flatten "properties" map into individual prefixed params
final Map<String, Object> propertiesMap = (Map<String, Object>) v1Params.remove(V2ApiConstants.PROPERTIES_KEY);
flattenMapWithPrefix(propertiesMap, v1Params, PROPERTY_PREFIX);

collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
}

@Command(name= V2_DELETE_ALIAS_CMD)
@SuppressWarnings("unchecked")
public void deleteAlias(PayloadObj<DeleteAliasPayload> obj) throws Exception {
final DeleteAliasPayload v2Body = obj.get();
final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
v1Params.put(ACTION, CollectionAction.DELETEALIAS.toLower());

collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
}

@Command(name = V2_CREATE_COLLECTION_CMD)
@SuppressWarnings("unchecked")
public void create(PayloadObj<CreatePayload> obj) throws Exception {
final CreatePayload v2Body = obj.get();
final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());

v1Params.put(ACTION, CollectionAction.CREATE.toLower());
convertV2CreateCollectionMapToV1ParamMap(v1Params);

collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
}

@SuppressWarnings("unchecked")
private void convertV2CreateCollectionMapToV1ParamMap(Map<String, Object> v2MapVals) {
// Keys are copied so that map can be modified as keys are looped through.
final Set<String> v2Keys = v2MapVals.keySet().stream().collect(Collectors.toSet());
for (String key : v2Keys) {
switch (key) {
case V2ApiConstants.PROPERTIES_KEY:
final Map<String, Object> propertiesMap = (Map<String, Object>) v2MapVals.remove(V2ApiConstants.PROPERTIES_KEY);
flattenMapWithPrefix(propertiesMap, v2MapVals, PROPERTY_PREFIX);
break;
case ROUTER_KEY:
final Map<String, Object> routerProperties = (Map<String, Object>) v2MapVals.remove(V2ApiConstants.ROUTER_KEY);
flattenMapWithPrefix(routerProperties, v2MapVals, CollectionAdminParams.ROUTER_PREFIX);
break;
case V2ApiConstants.CONFIG:
v2MapVals.put(CollectionAdminParams.COLL_CONF, v2MapVals.remove(V2ApiConstants.CONFIG));
break;
case V2ApiConstants.SHUFFLE_NODES:
v2MapVals.put(CollectionAdminParams.CREATE_NODE_SET_SHUFFLE_PARAM, v2MapVals.remove(V2ApiConstants.SHUFFLE_NODES));
break;
case V2ApiConstants.NODE_SET:
final Object nodeSetValUncast = v2MapVals.remove(V2ApiConstants.NODE_SET);
if (nodeSetValUncast instanceof String) {
v2MapVals.put(CollectionAdminParams.CREATE_NODE_SET_PARAM, nodeSetValUncast);
} else {
final List<String> nodeSetList = (List<String>) nodeSetValUncast;
final String nodeSetStr = String.join(",", nodeSetList);
v2MapVals.put(CollectionAdminParams.CREATE_NODE_SET_PARAM, nodeSetStr);
}
break;
default:
break;
}
}
}

private void flattenMapWithPrefix(Map<String, Object> toFlatten, Map<String, Object> destination,
String additionalPrefix) {
if (toFlatten == null || toFlatten.isEmpty() || destination == null) {
return;
}

toFlatten.forEach((k, v) -> destination.put(additionalPrefix + k, v));
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should make this recursive to support potentially nested v2 payloads?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually tried this initially but found the result to be a lot like the code in CollectionsApiMapping.Meta, which I personally find tough to read with all the map-literals for renaming properties and prefixing properties. I guessed it was more readable to just call this non-recursive version twice. That wouldn't work as well on APIs with deeper nesting, but seemed feasible here.

I'm open to going back that direction though if you prefer it. Did you have a particular signature in mind?

}
}

@EndPoint(path = {"/c/{collection}", "/collections/{collection}"},
method = DELETE,
permission = COLL_EDIT_PERM)
public void deleteCollection(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
req = ClusterAPI.wrapParams(req, "action",
req = wrapParams(req, ACTION,
CollectionAction.DELETE.toString(),
NAME, req.getPathTemplateValues().get(ZkStateReader.COLLECTION_PROP));
collectionsHandler.handleRequestBody(req, rsp);
Expand Down
8 changes: 4 additions & 4 deletions solr/core/src/java/org/apache/solr/servlet/QueryRateLimiter.java
View file
Open in desktop
Expand Up @@ -22,7 +22,7 @@

import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.request.beans.RateLimiterMeta;
import org.apache.solr.client.solrj.request.beans.RateLimiterPayload;
import org.apache.solr.common.cloud.SolrZkClient;
import org.apache.solr.common.cloud.ZkStateReader;
import org.apache.solr.common.util.Utils;
Expand Down Expand Up @@ -51,7 +51,7 @@ public void processConfigChange(Map<String, Object> properties) throws IOExcepti
return;
}

RateLimiterMeta rateLimiterMeta = mapper.readValue(configInput, RateLimiterMeta.class);
RateLimiterPayload rateLimiterMeta = mapper.readValue(configInput, RateLimiterPayload.class);

constructQueryRateLimiterConfigInternal(rateLimiterMeta, rateLimiterConfig);
}
Expand All @@ -74,7 +74,7 @@ private static RateLimiterConfig constructQueryRateLimiterConfig(SolrZkClient zk
return rateLimiterConfig;
}

RateLimiterMeta rateLimiterMeta = mapper.readValue(configInput, RateLimiterMeta.class);
RateLimiterPayload rateLimiterMeta = mapper.readValue(configInput, RateLimiterPayload.class);

constructQueryRateLimiterConfigInternal(rateLimiterMeta, rateLimiterConfig);

Expand All @@ -88,7 +88,7 @@ private static RateLimiterConfig constructQueryRateLimiterConfig(SolrZkClient zk
}
}

private static void constructQueryRateLimiterConfigInternal(RateLimiterMeta rateLimiterMeta, RateLimiterConfig rateLimiterConfig) {
private static void constructQueryRateLimiterConfigInternal(RateLimiterPayload rateLimiterMeta, RateLimiterConfig rateLimiterConfig) {

if (rateLimiterMeta == null) {
// No Rate limiter configuration defined in clusterprops.json
Expand Down
7 changes: 1 addition & 6 deletions solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java
View file
Open in desktop
Expand Up @@ -69,9 +69,7 @@ public void test() throws Exception {
} else {
req = CollectionAdminRequest.createCollection(COLLECTION_NAME, "conf1",2, 1, 0, 1);
}
setV2(req);
client.request(req);
assertV2CallsCount();
createCollection(null, COLLECTION_NAME1, 1, 1, client, null, "conf1");
}

Expand Down Expand Up @@ -414,8 +412,7 @@ private void clusterStatusWithCollection() throws IOException, SolrServerExcepti
private void clusterStatusZNodeVersion() throws Exception {
String cname = "clusterStatusZNodeVersion";
try (CloudSolrClient client = createCloudClient(null)) {
setV2(CollectionAdminRequest.createCollection(cname, "conf1", 1, 1)).process(client);
assertV2CallsCount();
CollectionAdminRequest.createCollection(cname, "conf1", 1, 1).process(client);
waitForRecoveriesToFinish(cname, true);

ModifiableSolrParams params = new ModifiableSolrParams();
Expand All @@ -438,9 +435,7 @@ private void clusterStatusZNodeVersion() throws Exception {
assertNotNull(znodeVersion);

CollectionAdminRequest.AddReplica addReplica = CollectionAdminRequest.addReplicaToShard(cname, "shard1");
setV2(addReplica);
addReplica.process(client);
assertV2CallsCount();
waitForRecoveriesToFinish(cname, true);

rsp = client.request(request);
Expand Down
4 changes: 3 additions & 1 deletion solr/core/src/test/org/apache/solr/handler/admin/TestCollectionAPIs.java
View file
Open in desktop
Expand Up @@ -84,7 +84,9 @@ public void testCommands() throws Exception {
ApiBag apiBag;
try (MockCollectionsHandler collectionsHandler = new MockCollectionsHandler()) {
apiBag = new ApiBag(false);
final CollectionsAPI collectionsAPI = new CollectionsAPI(collectionsHandler);
apiBag.registerObject(new CollectionsAPI(collectionsHandler));
apiBag.registerObject(collectionsAPI.collectionsCommands);
Collection<Api> apis = collectionsHandler.getApis();
for (Api api : apis) apiBag.register(api, Collections.emptyMap());

Expand Down Expand Up @@ -112,7 +114,7 @@ public void testCommands() throws Exception {


compareOutput(apiBag, "/collections", POST,
"{create-alias:{name: aliasName , collections:[c1,c2] }}", null, "{operation : createalias, name: aliasName, collections:[c1,c2] }");
"{create-alias:{name: aliasName , collections:[c1,c2] }}", null, "{operation : createalias, name: aliasName, collections:\"c1,c2\" }");

compareOutput(apiBag, "/collections", POST,
"{delete-alias:{ name: aliasName}}", null, "{operation : deletealias, name: aliasName}");
Expand Down