Make data streams in APIs resolvable. by martijnvg · Pull Request #54726 · elastic/elasticsearch

martijnvg · 2020-04-03T15:32:34Z

This change makes data stream resolvable in APIs.

The INCLUDE_DATA_STREAMS indices option controls whether data streams can be resolved in an api. In this pr, the INCLUDE_DATA_STREAMS indices option is enabled in the following APIs: search, msearch, refresh, index (op_type create only) and bulk (index requests with op type create only). In a subsequent later change, we will determine which other APIs need to be able to resolve data streams and enable the INCLUDE_DATA_STREAMS indices option for these APIs.

Whether an api resolve all backing indices of a data stream or the latest index of a data stream (write index) depends on the IndexNameExpressionResolver.Context.isResolveToWriteIndex().
If isResolveToWriteIndex() returns true then data streams resolve to the latest index (for example: index api) and otherwise a data stream resolves to all backing indices of a data stream (for example: search api).

Relates to #53100

This change makes data stream resolvable in all APIs. For APIs that have `FORBID_ALIASES_TO_MULTIPLE_INDICES` set in the index options, this means that a data stream is resolved to the latest backing index. For all other APIs this means that a data stream is resolved to all backing indices. Additionally also a `IGNORE_DATA_STREAMS` indices option is added, so that for specific APIs data stream resolvability can be disabled. Relates to elastic#53100

elasticmachine · 2020-04-03T15:32:36Z

Pinging @elastic/es-core-features (:Core/Features/Data streams)

…vable_in_apis

It is used in the Metadata class and otherwise node commands can't load the cluster state.

…vable_in_apis

…nds.

…vable_in_apis

to a change in the error message, this pr now returns `alias` with originates from the index abstraction type's display name.

… indices option. Data streams are not resolved unless the INCLUDE_DATA_STREAMS indices option is specified. Only search, msearch, index (with op type create) and refresh requests are configured with the INCLUDE_DATA_STREAMS indices option. All other requests / apis don't resolve data streams.

…vable_in_apis

danhermann

The latest PR updates LGTM

henningandersen · 2020-04-14T13:14:57Z

I think we should we add a few REST tests too that checks that doing index admin ops against a data-stream fails? Like we have also for aliases, for instance when deleting an index. I am not sure we need full coverage on that, but would be good to ensure a handful of most important cases are handled, like delete index, update settings, update mapping, GET index (should that fail, I think so?).

…vable_in_apis

…s is forbidden.

henningandersen

@martijnvg let me know your thoughts on moving the includeDataStreams option from IndicesOptions to IndexNameExpressionResolver and failing always when hitting data-stream (and not wanting to resolve them)?

server/src/main/java/org/elasticsearch/action/support/IndicesOptions.java

henningandersen · 2020-04-15T09:21:33Z

server/src/main/java/org/elasticsearch/action/support/IndicesOptions.java

@@ -133,12 +133,19 @@ public enum Option {
        new IndicesOptions(EnumSet.of(Option.ALLOW_NO_INDICES, Option.FORBID_CLOSED_INDICES),
            EnumSet.of(WildcardStates.OPEN, WildcardStates.HIDDEN));
    public static final IndicesOptions STRICT_EXPAND_OPEN_FORBID_CLOSED_IGNORE_THROTTLED =


If we stick to this approach, this should also be renamed?

henningandersen · 2020-04-15T09:32:51Z

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

+                    concreteIndices.add(writeIndex.getIndex());
+                }
            } else {
                if (indexAbstraction.getIndices().size() > 1 && !options.allowAliasesToMultipleIndices()) {


I think reusing this flag for data streams is not ideal. I wonder if we should instead flat out fail, if an expression matches a data-stream and we are not in one of the cases where data-streams are resolved.

I think I prefer this over skipping data-streams. Index, alias and data-stream are in the same namespace. I think it will be confusing to have some index admin operations that match indices and aliases, but ignores data-streams. Suppose you have:

xalias -> yindex
xindex
xdatastream

and you do POST /x*/_close, I think it would close yindex, xindex, but silently ignore xdatastream. Obviously, users should design their namings to avoid such scenarios, but I think a better default would be to fail, if an operation unexpectedly resolves to a data-stream.

Whether an operation or api is expected to expand a wildcard expression to a data stream that depends on whether that api is setup to resolve data streams. So in this case that would mean that ignoring data streams is expected behaviour. This is at least my expectation.

Henning and I chatted about this and we both now agree that the request should fail if data streams are not resolvable both the expression matches with one or more data streams. It makes it clear that an api doesn't support data streams (just like if a user specified a concrete data stream name) and this makes it easier to add data stream support if an api should support data streams.

henningandersen · 2020-04-15T10:59:15Z

server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java

        }
    }
+
+    public void testDataSteams() {


nit:

Suggested change

public void testDataSteams() {

public void testDataStreams() {

…vable_in_apis

…ssion expands to a data stream then fail. This means the api doesn't support data streams, but the wildcard expression is resolving to a data stream. Prior to this commit data streams would be quietly ignored. We think that failing is more clear, so that it is clear that a specific api call isn't executed over a data stream.

henningandersen

LGTM

henningandersen · 2020-04-16T16:12:50Z

server/src/main/java/org/elasticsearch/action/support/IndicesOptions.java

-        out.writeEnumSet(options);
+        if (out.getVersion().before(Version.V_8_0_0) && options.contains(Option.INCLUDE_DATA_STREAMS)) {
+            EnumSet<Option> copy = EnumSet.copyOf(options);
+            copy.remove(Option.INCLUDE_DATA_STREAMS);


I think we should put in a check into CreateDataStreamAction to not allow creating data streams before all nodes are on the target release (7.8)? To ensure that there are no surprises in a mixed cluster.

👍 I will make this change in a followup change.

Backport from: #54726 The INCLUDE_DATA_STREAMS indices option controls whether data streams can be resolved in an api for both concrete names and wildcard expressions. If data streams cannot be resolved then a 400 error is returned indicating that data streams cannot be used. In this pr, the INCLUDE_DATA_STREAMS indices option is enabled in the following APIs: search, msearch, refresh, index (op_type create only) and bulk (index requests with op type create only). In a subsequent later change, we will determine which other APIs need to be able to resolve data streams and enable the INCLUDE_DATA_STREAMS indices option for these APIs. Whether an api resolve all backing indices of a data stream or the latest index of a data stream (write index) depends on the IndexNameExpressionResolver.Context.isResolveToWriteIndex(). If isResolveToWriteIndex() returns true then data streams resolve to the latest index (for example: index api) and otherwise a data stream resolves to all backing indices of a data stream (for example: search api). Relates to #53100

Enabled data streams and itv2 feature enabled system properties in server module's integ test task. PR elastic#54726 added java integration tests for data streams, so this is why these system properties need to be enabled when running release build.

Enabled data streams and itv2 feature enabled system properties in server module's integ test task. PR #54726 added java integration tests for data streams, so this is why these system properties need to be enabled when running release build.

Enabled data streams and itv2 feature enabled system properties in server module's integ test task. PR elastic#54726 added java integration tests for data streams, so this is why these system properties need to be enabled when running release build.

) Enabled data streams and itv2 feature enabled system properties in server module's integ test task. PR #54726 added java integration tests for data streams, so this is why these system properties need to be enabled when running release build.

martijnvg added WIP :StorageEngine/Data streams Data streams and their lifecycles labels Apr 3, 2020

martijnvg added 12 commits April 3, 2020 18:18

iter

dc7e5c1

Merge remote-tracking branch 'es/master' into make_data_streams_resol…

2f70469

…vable_in_apis

fixed checkstyle errors.

8be36bc

fixed tests

559af0f

fixed forbidden api violation

aaf88c2

iter

fdef59c

fixed checkstyle violation

7b42797

iter

67a339f

added more tests

a418dbf

Merge remote-tracking branch 'es/master' into make_data_streams_resol…

84b0f5c

…vable_in_apis

fixed test

f4cc145

reuse test method

f3f96dd

martijnvg added v7.8.0 v8.0.0 >non-issue and removed WIP labels Apr 7, 2020

martijnvg added 4 commits April 7, 2020 11:51

altered test and fixed checkstyle violation

53e5400

Change node command so that it can parse, DataStreamMetadata.

4875ef6

It is used in the Metadata class and otherwise node commands can't load the cluster state.

added test

65cb204

Merge remote-tracking branch 'es/master' into make_data_streams_resol…

18382c4

…vable_in_apis

danhermann mentioned this pull request Apr 7, 2020

Provide index->data stream lookup #54929

Closed

martijnvg added 3 commits April 8, 2020 11:18

supply core custom metadata to namedXContentRegistry of es node comma…

4b289c5

…nds.

Merge remote-tracking branch 'es/master' into make_data_streams_resol…

c2482a8

…vable_in_apis

iter

5b76cc2

martijnvg marked this pull request as ready for review April 8, 2020 10:53

martijnvg requested a review from danhermann April 8, 2020 10:54

martijnvg added 6 commits April 9, 2020 13:36

if yaml test runs in a mixed cluster then Alias may be returned due

fc10fe2

to a change in the error message, this pr now returns `alias` with originates from the index abstraction type's display name.

clean data streams in test

a2c4556

fixed tests

af24175

Merge remote-tracking branch 'es/master' into make_data_streams_resol…

1b59cb2

…vable_in_apis

don't serialize include_datastreams if other node doesn't support it

1f91c80

martijnvg requested review from danhermann and henningandersen April 10, 2020 16:34

danhermann approved these changes Apr 13, 2020

View reviewed changes

martijnvg added 2 commits April 14, 2020 15:40

Merge remote-tracking branch 'es/master' into make_data_streams_resol…

9a604af

…vable_in_apis

add yaml tests that check that using a data stream in a number of api…

c04439f

…s is forbidden.

henningandersen reviewed Apr 15, 2020

View reviewed changes

martijnvg added 3 commits April 16, 2020 09:34

Merge remote-tracking branch 'es/master' into make_data_streams_resol…

8a256f3

…vable_in_apis

renamed test method

2544900

henningandersen approved these changes Apr 16, 2020

View reviewed changes

renamed constant

390f070

martijnvg merged commit fada09a into elastic:master Apr 16, 2020

martijnvg mentioned this pull request Apr 16, 2020

Backport: make data streams in APIs resolvable. #55337

Merged

martijnvg mentioned this pull request Apr 17, 2020

Enable feature enabled flags for java integration tests #55373

Merged

martijnvg mentioned this pull request Apr 17, 2020

Backport: enable feature enabled flags for java integration tests #55398

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

Make data streams in APIs resolvable.#54726

Make data streams in APIs resolvable.#54726
martijnvg merged 39 commits intoelastic:masterfrom
martijnvg:make_data_streams_resolvable_in_apis

martijnvg commented Apr 3, 2020 •

edited

Loading

Uh oh!

elasticmachine commented Apr 3, 2020

Uh oh!

danhermann left a comment

Uh oh!

henningandersen commented Apr 14, 2020

Uh oh!

henningandersen left a comment

Uh oh!

Uh oh!

henningandersen Apr 15, 2020

Uh oh!

henningandersen Apr 15, 2020

Uh oh!

martijnvg Apr 15, 2020

Uh oh!

martijnvg Apr 16, 2020

Uh oh!

henningandersen Apr 15, 2020

Uh oh!

henningandersen left a comment

Uh oh!

henningandersen Apr 16, 2020

Uh oh!

martijnvg Apr 16, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	public void testDataSteams() {
	public void testDataStreams() {

Conversation

martijnvg commented Apr 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Apr 3, 2020

Uh oh!

danhermann left a comment

Choose a reason for hiding this comment

Uh oh!

henningandersen commented Apr 14, 2020

Uh oh!

henningandersen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

henningandersen Apr 15, 2020

Choose a reason for hiding this comment

Uh oh!

henningandersen Apr 15, 2020

Choose a reason for hiding this comment

Uh oh!

martijnvg Apr 15, 2020

Choose a reason for hiding this comment

Uh oh!

martijnvg Apr 16, 2020

Choose a reason for hiding this comment

Uh oh!

henningandersen Apr 15, 2020

Choose a reason for hiding this comment

Uh oh!

henningandersen left a comment

Choose a reason for hiding this comment

Uh oh!

henningandersen Apr 16, 2020

Choose a reason for hiding this comment

Uh oh!

martijnvg Apr 16, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

martijnvg commented Apr 3, 2020 •

edited

Loading