elastic / elasticsearch

Free and Open Source, Distributed, RESTful Search Engine
https://www.elastic.co/products/elasticsearch
Other
1.1k stars 24.83k forks source link

[CCR] Change APIs to align better with how CCR will be used #33931

Closed martijnvg closed 6 years ago

martijnvg commented 6 years ago

The names of the current CCR APIs don't match well with what they are doing and the use cases CCR needs to support.

Based on what ccr does and the use cases it needs support there is a need for the following primitives:

  1. Create the follow index based on settings and the mapping in the leader index.
  2. Start fetching of write operations from leader and replicate into follow index.
  3. Stop the fetching of write operations from the leader and replication into follow index.
  4. Transform a follow index into a normal index. After this step it cannot become a follower index.

It should be possible to stop and than start following a leader index with the same follow index, so that parameters that control certain characteristics of the persistent tasks following leader shards can be changed to improve replication throughput.

Currently there are the following APIs:

The follow changes should be made in order to better align CCR with the supported use cases:

  1. Rename the following apis:

    • follow api > start api (/{index}/_ccr/follow > /{index}/_ccr/start)
    • unfollow api > stop api (/{index}/_ccr/unfollow > /{index}/_ccr/stop)
  2. Add a new api, named the unfollow api, that changes a follower index into a regular index, so that it can handle regular write operations. This API will require the follow index to be closed. It will remove the index.xpack.ccr.following_index setting and ccr custom index metadata in the IndexMetaData of the follow index. After this; when the index is opened then it will not longer use the FollowingEngine.

elasticmachine commented 6 years ago

Pinging @elastic/es-distributed

bleskes commented 6 years ago

+1

jasontedor commented 6 years ago

I think that follow should appear in the start and stop APIs. When I look at the endpoints /{index}/_ccr/start and /{index}/_ccr/stop I think we should be explicit about what is being started and stopped, namely following, otherwise I think the intent is not clear. Thus, I think that follow should appear in these endpoints:

Additionally, I am not sure if start and stop are the right verbs. I think of start as beginning following as opposed to resuming following which this endpoint can be used for. Maybe these endpoints should be:

Next, maybe create_and_follow does not need to be so verbose. How about simply follow with the semantics that it creates the index and starts following? I realize this is slightly contrary to my point in the previous API where I felt that we needed to be explicit about what we are starting and stopping, but I don't see the need to be explicit that we are creating the index here in the verbs in the API, I think we can get away with just follow. Then we have a nice symmetry:

with endpoints:

What do you think @bleskes @dliappis @dnhatn @martijnvg?

bleskes commented 6 years ago

I'm good with pause/resume instead of stop/start. IMO we don't need the follow about I'm ok with having it.

Re /{index}/_ccr/follow - I'm concerned with this one. I'm worried that if we in future introduce an api that does allow you to take an existing index and tell it to follow another index (where it makes senses) we'll be in trouble. Creating an index is a major event - I like it's in the api name.

martijnvg commented 6 years ago

Re /{index}/_ccr/follow - I'm concerned with this one. I'm worried that if we in future introduce an api that does allow you to take an existing index and tell it to follow another index (where it makes senses) we'll be in trouble.

If we would make the follow api (which is now create and follow) only accept a http PUT and when we add the ability to turn an existing index into a follow index we also accept a http POST then would that be clear enough? The PUT variant would fail if the follow index already exists and the POST variant would fail if the follow index is missing.

I'm 👍 with pause/resume instead of stop/start and the reasoning behind it. Personally I like the symmetry that comes with the shortening from /{index}/_ccr/create_and_follow to just /{index}/_ccr/follow, but I'm ok with sticking to create and follow if others think it is clearer.

jasontedor commented 6 years ago

@bleskes I had the same thought as you this morning regarding future APIs and had the same thought as @martijnvg as a solution to it: PUT for create and POST for an existing index. Is this satisfactory to you @bleskes?

bleskes commented 6 years ago

I feel the difference between POST and PUT is too subtle and many people make mistakes. I don't feel really strongly about it so the group prefers this much better, I will go along.

bleskes commented 6 years ago

@elastic/es-clients any opinions here?

Mpdreamz commented 6 years ago

What are the semantics behind pause/resume? If resume picks up right where pause left off then definitely agree pause/resume are better verbs then start/stop.

I agree with @boaz that the difference is a bit too subtle. If we reuse the /{index}/_ccr/follow for both PUT and POST they should at least be documented as two separate API's i.e ccr.create_and_follow.json and ccr.follow.json

There are other API's that accept both PUT and POST without changing semantics such as snapshot.create.json and the one that does index.json has active discussions to split into two explicit different API's as well.

jasontedor commented 6 years ago

Thanks for the feedback everyone.

I feel that our proposed use of PUT and POST here is fairly conventional, and it doesn't seem the feelings against are strong (please correct me if I am reading this wrongly). We will indeed document these as separate APIs @Mpdreamz!

So let us move forward with:

PUT /{index}/_ccr/follow <-- create a new follower index
POST /{index}/_ccr/pause_follow <-- pause a follower index
POST /{index}/_ccr/resume_follow <-- resume a follower index
POST /{index}/_ccr/unfollow <-- convert a follower index to a regular index

and reserving but not currently implementing

POST /{index}/_ccr/follow <-- convert an existing index to a follower index