Skip to content

SOLR-16392: Convert v2 shard, replica deletion to JAX-RS #1594

Merged
gerlowskija merged 8 commits intoapache:mainfrom
gerlowskija:SOLR-16392-delete-replica-shard-api-conversions
May 10, 2023
Merged

SOLR-16392: Convert v2 shard, replica deletion to JAX-RS #1594
gerlowskija merged 8 commits intoapache:mainfrom
gerlowskija:SOLR-16392-delete-replica-shard-api-conversions

Conversation

@gerlowskija
Copy link
Contributor

@gerlowskija gerlowskija commented Apr 27, 2023

https://issues.apache.org/jira/browse/SOLR-16392

Description

Solr is slowly migrating its v2 API definitions over to the JAX-RS framework. Some APIs have already been converted, but many remain: including the APIs for replica and shard deletion.

Solution

This PR migrates the v2 delete-replica and delete-shard APIs over to JAX-RS. No user-visible changes were made to the v2 delete-shard endpoint. Delete-replica however has been split into two separate endpoints (one to delete a single named replica, and one to delete a specified number of (unnamed) replicas. The v2 APIs now appear as below:

  • "Delete shard" - DELETE /api/collections/collName/shards/shardName (no changes)
  • "Delete named replica" - DELETE /api/collections/collName/shards/shardName/replicas/replicaName
  • "Delete multiple replicas from specific shard" - DELETE /api/collections/collName/shards/shardName/replicas?count=3
  • "Delete multiple replicas from all shards" - PUT /api/collections/collName/scale {"count": 3}

Tests

See DeleteReplicaAPITest and DeleteShardAPITest

Checklist

Please review the following and check all that apply:

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
  • I have developed this patch against the main branch.
  • I have run ./gradlew check.
  • I have added tests for my changes.
  • I have added documentation for the Reference Guide

@gerlowskija
Copy link
Contributor Author

gerlowskija commented Apr 27, 2023

You learn something new every day: it turns out that DELETEREPLICA covers three distinct use-cases:

  1. Delete a single named replica (DELETE /api/collections/collName/shards/shardName/replicas/replicaName)
  2. Delete multiple replicas from a single shard (DELETE /api/collections/collName/shards/shardName/replicas?count=123)
  3. Delete multiple replicas from all shards (???)

(1) and (2) are covered by the v2 APIs mentioned inline, but I'm struggling to come up with a REST-ful v2 API to cover that last scenario. DELETE /api/collections/cN/shards/replicas?count=123 is out because it's indistinguishable from a user attempting to delete a shard that they named "replicas".

Anyone have any ideas?

@HoustonPutman
Copy link
Contributor

Maybe we don't count it as a delete anymore, when the action is more of a scale. So PUT /api/collections/Cn/replicas/scale?count=123. This would cover scaling up or scaling down replicas. If we support scaling shards, which I don't think we do, we could do the same thing under PUT /api/collections/Cn/shards/scale. (I don't think another API uses PUT with that shards path)

@gerlowskija
Copy link
Contributor Author

Maybe we don't count it as a delete anymore, when the action is more of a scale. So PUT /api/collections/Cn/replicas/scale?count=123

Thanks for the suggestion!

I have some REST-fulness quibbles: having a "replicas" path that doesn't live under /shards/Sn might be confusing to users who are familiar with the /collections/Cn/shards/Sn/replicas/Rn pattern we've established with other APIs. But it's definitely the best option at this point IMO!

Thinking aloud a bit, maybe we keep the idea of this being a "scaling" API, but drop the "replicas" path segment. e.g PUT /api/collections/Cn/scale

Longer term, maybe this behavior could even be folded into MODIFYCOLLECTION: when someone does a "modify" (PUT /api/collections/Cn {...}) which happens to change nrtReplicas, etc, Solr would do the scaling for them. Though of course that'd be a larger change and not something to tackle here...

@gerlowskija
Copy link
Contributor Author

For now, I've ended up going with PUT /api/collections/cN/scale {"count": 3} for the "delete-N-replicas-from-all-shards" functionality. I'm definitely not in love with it though, and hope that someone comes up with a better option as time goes on. All of the v2 APIs are "experimental" at this point, so we're free to change them as needed.

@gerlowskija gerlowskija merged commit a93a25b into apache:main May 10, 2023
@gerlowskija gerlowskija deleted the SOLR-16392-delete-replica-shard-api-conversions branch May 10, 2023 12:55
gerlowskija added a commit that referenced this pull request May 16, 2023
This commit tweaks the v2 binding for our "delete replica" API to be more
intuitive for users.  Delete replica functionality now lives at:
  - `DELETE /api/collections/cName/shards/sName/replicas/rName`
  - `DELETE /api/collections/cName/shards/sName/replicas?count=3`
  - `PUT /api/collections/cName/scale {"count": 3"}

depending on whether users want to delete a single replica by name,
multiple replicas from a single shard, or multiple replicas from all shards
respectively.  These v2 APIs are experimental and currently subject to
change.

This commit also switches them (and a similar deleteshard API) over to
using JAX-RS.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments