Add index block api #58094

ywelsch · 2020-06-15T09:01:52Z

Adds an API for putting an index block in place, which also ensures for write blocks that, once successfully returning to the user, all shards of the index are properly accounting for the block, for example that all in-flight writes to an index have been completed after adding the write block.

This API allows coordinating more complex workflows, where it is crucial that an index is no longer receiving writes after the API completes, useful for example when marking an index as read-only during an upgrade in order to reindex its documents.

Note to reviewers: I will fix the docs in a later revision once we're all good on naming etc.
Also, HLRC support will only come later.

elasticmachine · 2020-06-17T08:26:04Z

Pinging @elastic/es-core-features (:Core/Features/Indices APIs)

dakrone

Thanks for adding this Yannick, I think this will be a neat API and it'll be nice to be able to use this from ILM. I left some comments for you. I have two additional thoughts:

I think we should change /{index}/_blocks/{block} to /{index}/_block/{block}. By and large the bulk of our APIs use singular verbs/nouns, and it reads better mentally (/foo/_blocks/read can be thought of as "foo blocks reads", but /foo/_blocks/read_only is awkward thought of as "foo blocks read onlys", it's nicer to think of /foo/_block/read_only as "add the read_only block to foo"). It's also useful because we only support a single block, not a comma-separated or wildcard of blocks to add to the index.
How do these blocks work for a searchable snapshot index? Should we add a test for that? (the write block is pretty obviously not needed, but a read block may still be desired)

server/src/internalClusterTest/java/org/elasticsearch/blocks/SimpleBlocksIT.java

server/src/main/java/org/elasticsearch/action/admin/indices/readonly/AddIndexBlockAction.java

.../org/elasticsearch/action/admin/indices/readonly/AddIndexBlockClusterStateUpdateRequest.java

server/src/main/java/org/elasticsearch/action/admin/indices/readonly/AddIndexBlockRequest.java

server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexStateService.java

ywelsch · 2020-06-24T09:54:11Z

@dakrone I've adapted /{index}/_blocks/{block} to /{index}/_block/{block} according to your suggestion. Regarding the interoperability with searchable snapshots: The API and verify action here is completely agnostic to the underlying engine being used, so will just work with frozen / searchable snapshots / whatever other flavor we have there. I will comment in-line on the other stuff.

ywelsch · 2020-06-24T10:08:20Z

@jrodewig can you help out with the docs here? (Feel free to directly push changes to this PR)

jrodewig

@ywelsch I made some changes to fix the docs CI test. However, I left some other feedback that should be addressed.

docs/reference/index-modules/blocks.asciidoc

jrodewig

:doh: Forgot you gave me permission to push the changes. I went ahead and added my suggestions. 😄

dakrone

Thanks for the update Yannick, I made some more comments.

As an aside, it doesn't appear to me (though I am certainly not an expert in shard operation permits!) that this allows adding a read block and waiting for all query operations to finish before returning. Is that correct? Out of curiosity, how hard would that be to add? (It wouldn't have to be in this PR) It would be very useful for ILM operations if we had that ability

rest-api-spec/src/main/resources/rest-api-spec/api/indices.add_block.json

dakrone · 2020-06-25T22:48:50Z

server/src/main/java/org/elasticsearch/action/admin/indices/readonly/AddIndexBlockResponse.java

+
+        @Override
+        public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException {
+            builder.startObject(index.getName());


I don't agree, I think if we go with consistency with our earlier APIs, then we would never eventually transition to the change. Since this is introducing a new API if we make the change now it's one less that we have to change in the future when we eventually have REST versioning.

We've discussed this a lot on the core/features team and always ended up with newly introduced APIs using the newer format, rather than matching the existing dynamically keyed format.

.../main/java/org/elasticsearch/action/admin/indices/readonly/TransportAddIndexBlockAction.java

...va/org/elasticsearch/action/admin/indices/readonly/TransportVerifyShardIndexBlockAction.java

server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexStateService.java

dakrone · 2020-06-25T23:10:58Z

docs/reference/index-modules/blocks.asciidoc

+index settings, or can be added using a dedicated API, which also ensures
+that, once successfully returning to the user, all shards of the index are
+properly accounting for the block, for example that all in-flight writes to
+an index have been completed after adding the write block.


Should we call out that this only applies to write operations, not read operations?

ywelsch · 2020-06-29T08:18:39Z

As an aside, it doesn't appear to me (though I am certainly not an expert in shard operation permits!) that this allows adding a read block and waiting for all query operations to finish before returning. Is that correct?

You're absolutely right here. The PR was overpromising in this regard (I initially started with a dedicated API to just allow adding a write block, which eventually got turned into this more general one here). I've adapted the docs to point out that we only guarantee this for write operations.

Out of curiosity, how hard would that be to add? (It wouldn't have to be in this PR) It would be very useful for ILM operations if we had that ability

We could do something similar for reads as we do for writes, but there are a bigger number of cases / scenarios to consider and to handle accordingly. For example, how should his affect scrolls that have been opened before the block has been put in place (AFAICS we don't directly close the scroll context today). Similarly how should this handle long-running searches, or a fetch phase (where the query phase successfully executed)...

dakrone

LGTM, thanks for iterating on it Yannick!

For example, how should his affect scrolls that have been opened before the block has been put in place (AFAICS we don't directly close the scroll context today). Similarly how should this handle long-running searches, or a fetch phase (where the query phase successfully executed)...

This is the exact scenario I was thinking about, in some places in ILM (like after a shrink) we do a swap-and-delete from the alias, but we have some unavoidable issues where the query may be in the middle of execution during the delete, we unfortunately don't have a way to wait until they are all done. Maybe in the future though?

ywelsch · 2020-06-29T15:35:07Z

in some places in ILM (like after a shrink) we do a swap-and-delete from the alias, but we have some unavoidable issues where the query may be in the middle of execution during the delete, we unfortunately don't have a way to wait until they are all done.

I'm wondering if that's a situation which should rather be covered by a more resilient _search API, where it would retry searching on the new shards (after alias been swapped), similar to what's discussed in #56236

ywelsch · 2020-06-30T07:33:31Z

Thanks @dakrone and @jrodewig

Adds an API for putting an index block in place, which also ensures for write blocks that, once successfully returning to the user, all shards of the index are properly accounting for the block, for example that all in-flight writes to an index have been completed after adding the write block. This API allows coordinating more complex workflows, where it is crucial that an index is no longer receiving writes after the API completes, useful for example when marking an index as read-only during an upgrade in order to reindex its documents.

The read-only-allow-delete block is not really under the user's control since Elasticsearch adds/removes it automatically. This commit removes support for it from the new API for adding blocks to indices that was introduced in elastic#58094.

* Forbid read-only-allow-delete block in blocks API The read-only-allow-delete block is not really under the user's control since Elasticsearch adds/removes it automatically. This commit removes support for it from the new API for adding blocks to indices that was introduced in #58094. * Missing xref * Reword paragraph on read-only-allow-delete block

The read-only-allow-delete block is not really under the user's control since Elasticsearch adds/removes it automatically. This commit removes support for it from the new API for adding blocks to indices that was introduced in #58094.

ywelsch added 11 commits June 12, 2020 14:43

Add API for adding index blocks

ed6f7e5

Towards REST API

634bb38

Make metadatta block idempotent as well

67511f8

more REST test changes

877b465

Add docs

f2c1934

No HLRC yet

66be434

fix test

31dbfea

Merge remote-tracking branch 'elastic/master' into add-index-block-api

0a5c01d

chstyl

0446032

fix test

5ba6928

Get basic stuff right

a876465

ywelsch added v7.9.0 v8.0.0 :Data Management/Indices APIs APIs to create and manage indices and templates >enhancement labels Jun 17, 2020

ywelsch marked this pull request as ready for review June 17, 2020 08:26

elasticmachine added the Team:Data Management Meta label for data/management team label Jun 17, 2020

dakrone self-requested a review June 17, 2020 14:25

dakrone requested changes Jun 17, 2020

View reviewed changes

ywelsch added 5 commits June 23, 2020 10:46

Merge remote-tracking branch 'elastic/master' into add-index-block-api

558c3b8

Use singular

db583d4

Review comments

97a6e93

Merge remote-tracking branch 'elastic/master' into add-index-block-api

22e0195

checkstyleo

85ab5be

change action name

6d23195

ywelsch requested a review from dakrone June 24, 2020 10:08

fix test

27fa800

ywelsch and others added 2 commits June 24, 2020 12:43

One more occurrence of close

6953569

Fix docs CI

39498f6

jrodewig reviewed Jun 24, 2020

View reviewed changes

Additional doc changes

a306955

jrodewig approved these changes Jun 24, 2020

View reviewed changes

dakrone requested changes Jun 25, 2020

View reviewed changes

ywelsch added 4 commits June 29, 2020 09:32

Merge remote-tracking branch 'elastic/master' into add-index-block-api

e20b9f7

Nits

dff8526

Only for write blocks

1f9ccb0

API

0f39c54

touched wrong class

0bf1c91

dakrone approved these changes Jun 29, 2020

View reviewed changes

dakrone mentioned this pull request Jun 29, 2020

Switch ILM read only blocking to use the index block API #58678

Closed

ywelsch merged commit 5e345e1 into elastic:master Jun 30, 2020

ywelsch mentioned this pull request Jun 30, 2020

Add index block api #58716

Merged

DaveCTurner mentioned this pull request Jun 30, 2020

Forbid read-only-allow-delete block in blocks API #58727

Merged

ywelsch added a commit that referenced this pull request Jun 30, 2020

Adapt BWC after backport of (#58094)

e4df928

russcam mentioned this pull request Jul 23, 2020

7.9.0 Meta ticket elastic/elasticsearch-net#4872

Closed

29 tasks

joegallo mentioned this pull request Oct 30, 2020

Reimplement the readonly action in terms of the index block API #64439

Merged

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add index block api #58094

Add index block api #58094

ywelsch commented Jun 15, 2020 •

edited

Loading

elasticmachine commented Jun 17, 2020

dakrone left a comment •

edited

Loading

ywelsch commented Jun 24, 2020

ywelsch commented Jun 24, 2020

jrodewig left a comment

jrodewig left a comment •

edited

Loading

dakrone left a comment

dakrone Jun 25, 2020

dakrone Jun 25, 2020

ywelsch commented Jun 29, 2020

dakrone left a comment

ywelsch commented Jun 29, 2020

ywelsch commented Jun 30, 2020

Add index block api #58094

Add index block api #58094

Conversation

ywelsch commented Jun 15, 2020 • edited Loading

elasticmachine commented Jun 17, 2020

dakrone left a comment • edited Loading

Choose a reason for hiding this comment

ywelsch commented Jun 24, 2020

ywelsch commented Jun 24, 2020

jrodewig left a comment

Choose a reason for hiding this comment

jrodewig left a comment • edited Loading

Choose a reason for hiding this comment

dakrone left a comment

Choose a reason for hiding this comment

dakrone Jun 25, 2020

Choose a reason for hiding this comment

dakrone Jun 25, 2020

Choose a reason for hiding this comment

ywelsch commented Jun 29, 2020

dakrone left a comment

Choose a reason for hiding this comment

ywelsch commented Jun 29, 2020

ywelsch commented Jun 30, 2020

ywelsch commented Jun 15, 2020 •

edited

Loading

dakrone left a comment •

edited

Loading

jrodewig left a comment •

edited

Loading