Allow a number of broadcast transport actions to resolve data streams by martijnvg · Pull Request #55726 · elastic/elasticsearch

martijnvg · 2020-04-24T14:54:17Z

Change TransportBroadcastByNodeAction and TransportBroadcastReplicationAction
to be able to resolve data streams by default. Implementations can change this ability.

This change allows to following APIs to resolve data streams: flush,
refresh (already supported data streams), force merge, clear indices cache,
indices stats, segments, upgrade stats, upgrade, validate query,
searchable snapshots stats, clear searchable snapshots cache and
reload analyzers APIs.

Relates to #53100

…tream By changing to the default indices options of BroadcastRequest the following apis can resolve data streams: flush, refresh (already supported data streams), force merge, clear indices cache, indices stats, segments, upgrade stats, upgrade, validate query, searchable snapshots stats, clear searchable snapshots cache and reload analyzers APIs. The following APIs extend BroadcastRequest but overwrite default indices options and therefor don't yet support data streams: recovery api This change also adds `strictIncludeDataStreamsExpand()` to `IndicesOptions` class. (an alternative to `strictExpand()`) This is because `InternalClusterInfoService` overwrites the default indices options for the indices stats call. Relates to elastic#53100

elasticmachine · 2020-04-24T14:54:19Z

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

danhermann

LGTM. Left one minor comment

danhermann · 2020-04-24T21:29:38Z

+     * @return indices option that requires every specified index to exist, expands wildcards to both open and closed
+     * indices and allows that no indices are resolved from wildcard expressions (not returning an error).


Maybe add that data streams are included to distinguish it from ::strictExpand()

…ty_BroadcastRequest

henningandersen · 2020-04-27T08:43:03Z

I think ForgetFollowerAction.Request also cannot meaningfully resolve data streams.

I am also a bit worried that we here are introducing support for data streams in a wide array of APIs without any tests for it. I think it would be better to tackle them one by one, changing the indices options in each specific request and adding the necessary tests for it?

martijnvg · 2020-04-28T06:52:18Z

I think ForgetFollowerAction.Request also cannot meaningfully resolve data streams.

True and it doesn't support aliases and wildcard expressions too. The corresponding transport action uses the leader and follower indices as provided in the api. Which why this PR doesn't add data stream support to this api either. The ForgetFollowerAction.Request supplies the leader index to the base class (BroadcastRequest) and that is then used to find the leader shards, but in the transport action's shardOperation() method it then uses the leader index as is provided via api. Using using anything other than the concrete leader index results in failure. I think this behaviour is intentional.

I think it would be better to tackle them one by one, changing the indices options in each specific request and adding the necessary tests for it?

I've added tests for all the APIs that have a request that extends from BroadcastRequest and that support data streams. So that should be ok? Or do you prefer that I revert the change to BroadcastRequest and make the change to all the concrete request classes that should support data streams? (similar to the refresh request class prior to this PR)

to include data streams by default. If a wildcard expression (which is `*` here) matches a data stream but the indices options don't include data streams then an error is returned. This change was spawned of from elastic#55726 which changes the default indices options of the indices stats api. This will avoid cluster info service from failing to refresh the stats: ``` [2020-04-29T17:16:00,218][WARN ][o.e.c.InternalClusterInfoService] [runTask-0] Failed to execute IndicesStatsAction for ClusterInfoUpdateJob java.lang.IllegalArgumentException: The provided expression [[_all]] matches a data stream, specify the corresponding concrete indices instead. at org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.dataStreamsNotSupportedException(IndexNameExpressionResolver.java:278) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.cluster.metadata.IndexNameExpressionResolver$WildcardExpressionResolver.resolve(IndexNameExpressionResolver.java:699) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.concreteIndices(IndexNameExpressionResolver.java:159) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.concreteIndexNames(IndexNameExpressionResolver.java:138) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.concreteIndexNames(IndexNameExpressionResolver.java:71) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.action.support.broadcast.node.TransportBroadcastByNodeAction$AsyncAction.<init>(TransportBroadcastByNodeAction.java:252) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.action.support.broadcast.node.TransportBroadcastByNodeAction.doExecute(TransportBroadcastByNodeAction.java:225) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.action.support.broadcast.node.TransportBroadcastByNodeAction.doExecute(TransportBroadcastByNodeAction.java:76) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.action.support.TransportAction$RequestFilterChain.proceed(TransportAction.java:88) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.action.support.ActionFilter$Simple.apply(ActionFilter.java:53) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.action.support.TransportAction$RequestFilterChain.proceed(TransportAction.java:86) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.xpack.security.action.filter.SecurityActionFilter.lambda$apply$0(SecurityActionFilter.java:86) [x-pack-security-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.action.ActionListener$1.onResponse(ActionListener.java:63) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.xpack.security.action.filter.SecurityActionFilter.lambda$authorizeRequest$4(SecurityActionFilter.java:172) [x-pack-security-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.action.ActionListener$1.onResponse(ActionListener.java:63) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.xpack.security.authz.AuthorizationService.authorizeSystemUser(AuthorizationService.java:388) [x-pack-security-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.xpack.security.authz.AuthorizationService.authorize(AuthorizationService.java:192) [x-pack-security-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.xpack.security.action.filter.SecurityActionFilter.authorizeRequest(SecurityActionFilter.java:172) [x-pack-security-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.xpack.security.action.filter.SecurityActionFilter.lambda$applyInternal$3(SecurityActionFilter.java:158) [x-pack-security-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.action.ActionListener$1.onResponse(ActionListener.java:63) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.xpack.security.authc.AuthenticationService$Authenticator.lambda$authenticateAsync$2(AuthenticationService.java:323) [x-pack-security-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.xpack.security.authc.AuthenticationService$Authenticator.lambda$lookForExistingAuthentication$6(AuthenticationService.java:385) [x-pack-security-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.xpack.security.authc.AuthenticationService$Authenticator.lookForExistingAuthentication(AuthenticationService.java:396) [x-pack-security-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.xpack.security.authc.AuthenticationService$Authenticator.authenticateAsync(AuthenticationService.java:320) [x-pack-security-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.xpack.security.authc.AuthenticationService.authenticate(AuthenticationService.java:156) [x-pack-security-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.xpack.security.action.filter.SecurityActionFilter.applyInternal(SecurityActionFilter.java:155) [x-pack-security-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.xpack.security.action.filter.SecurityActionFilter.lambda$apply$1(SecurityActionFilter.java:92) [x-pack-security-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.xpack.core.security.SecurityContext.executeAsUser(SecurityContext.java:127) [x-pack-core-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.xpack.security.action.filter.SecurityActionFilter.apply(SecurityActionFilter.java:90) [x-pack-security-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.action.support.TransportAction$RequestFilterChain.proceed(TransportAction.java:86) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.action.support.TransportAction.execute(TransportAction.java:64) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.tasks.TaskManager.registerAndExecute(TaskManager.java:151) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.client.node.NodeClient.executeLocally(NodeClient.java:90) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.client.node.NodeClient.doExecute(NodeClient.java:79) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.client.support.AbstractClient.execute(AbstractClient.java:380) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.client.support.AbstractClient$IndicesAdmin.execute(AbstractClient.java:1210) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.client.support.AbstractClient$IndicesAdmin.stats(AbstractClient.java:1465) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] at org.elasticsearch.cluster.InternalClusterInfoService.updateIndicesStats(InternalClusterInfoService.java:264) [elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT] ```

henningandersen · 2020-04-30T16:08:19Z

About ForgetFollowerAction, I think it will result in an NPE

        final Index leaderIndex = clusterService.state().metadata().index(request.leaderIndex()).getIndex();

I think we should rather use an indicesOptions that does not support data streams. I think the investigation work you are doing will likely address this?

I think the tests look fine (though the forget follower action is missing). Thanks for pointing that out.

I wonder if I should review this now or wait until you concluded the experiment on extracting the data stream option?

martijnvg · 2020-04-30T18:46:13Z

I wonder if I should review this now or wait until you concluded the experiment on extracting the data stream option?

Yes, I think we should wait with this pr and see how #56034 unfolds.

…ty_BroadcastRequest

martijnvg · 2020-05-08T15:16:41Z

I've updated this PR based on the changes that came with #56034.

henningandersen

LGTM.

…elastic#55726) Change TransportBroadcastByNodeAction and TransportBroadcastReplicationAction to be able to resolve data streams by default. Implementations can change this ability. This change allows to following APIs to resolve data streams: flush, refresh (already supported data streams), force merge, clear indices cache, indices stats (already supported data streams), segments, upgrade stats, upgrade, validate query, searchable snapshots stats, clear searchable snapshots cache and reload analyzers APIs. Relates to elastic#53100

…#55726) (#56502) Change TransportBroadcastByNodeAction and TransportBroadcastReplicationAction to be able to resolve data streams by default. Implementations can change this ability. This change allows to following APIs to resolve data streams: flush, refresh (already supported data streams), force merge, clear indices cache, indices stats (already supported data streams), segments, upgrade stats, upgrade, validate query, searchable snapshots stats, clear searchable snapshots cache and reload analyzers APIs. Relates to #53100

martijnvg added >non-issue v8.0.0 :StorageEngine/Data streams Data streams and their lifecycles v7.8.0 labels Apr 24, 2020

martijnvg added 2 commits April 24, 2020 16:55

renamed test

bbe6a32

fixed assertion

9a7406c

martijnvg marked this pull request as draft April 24, 2020 15:05

martijnvg requested review from danhermann and henningandersen April 24, 2020 15:40

martijnvg marked this pull request as ready for review April 24, 2020 15:40

martijnvg added 2 commits April 24, 2020 18:10

adjusted tests

6eee5ad

fixed typo

61b81ca

danhermann approved these changes Apr 24, 2020

View reviewed changes

martijnvg added 2 commits April 25, 2020 12:11

tweaked jdoc

911f928

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

b1e176e

…ty_BroadcastRequest

martijnvg mentioned this pull request Apr 29, 2020

Change cluster info service indices stats request to include data streams by default #55951

Closed

martijnvg mentioned this pull request Apr 30, 2020

Move includeDataStream flag from IndicesOptions to IndexNameExpressionResolver.Context #56034

Merged

rjernst added the Team:Data Management (obsolete) DO NOT USE. This team no longer exists. label May 4, 2020

pugnascotia added v7.8.1 v7.9.0 and removed v7.8.0 v7.8.1 labels May 6, 2020

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

2253302

…ty_BroadcastRequest

martijnvg added 2 commits May 8, 2020 13:31

changes after merge

3413acb

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

55ef7b7

…ty_BroadcastRequest

martijnvg changed the title ~~Change the BroadcastRequest default indices options to resolve data stream~~ Allow a number of broadcast transport actions to resolve data streams May 8, 2020

forget follower action should not resolve data streams

0c41214

henningandersen approved these changes May 11, 2020

View reviewed changes

martijnvg merged commit 77aa236 into elastic:master May 11, 2020

martijnvg mentioned this pull request May 11, 2020

Backport: allow a number of broadcast transport actions to resolve data streams #56502

Merged

This was referenced Jul 2, 2020

[DOCS] Add data streams to flush API docs #58950

Merged

[DOCS] Add data streams to force merge API docs #58951

Merged

[DOCS] Add data streams to clear cache API docs #59324

Merged

[DOCS] Add data streams to searchable snapshot API docs #59325

Merged

This was referenced Jul 9, 2020

[DOCS] Add data streams to field caps API docs #59326

Merged

[DOCS] Add data streams to index APIs #59329

Merged

[DOCS] Add data streams to validate query API #59420

Merged

[DOCS] Add data streams to reload search analyzers API #59422

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

Allow a number of broadcast transport actions to resolve data streams#55726

Allow a number of broadcast transport actions to resolve data streams#55726
martijnvg merged 11 commits intoelastic:masterfrom
martijnvg:data_stream_resolvability_BroadcastRequest

martijnvg commented Apr 24, 2020 •

edited

Loading

Uh oh!

elasticmachine commented Apr 24, 2020

Uh oh!

danhermann left a comment

Uh oh!

danhermann Apr 24, 2020

Uh oh!

henningandersen commented Apr 27, 2020

Uh oh!

martijnvg commented Apr 28, 2020

Uh oh!

henningandersen commented Apr 30, 2020

Uh oh!

martijnvg commented Apr 30, 2020

Uh oh!

martijnvg commented May 8, 2020

Uh oh!

henningandersen left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

		* @return indices option that requires every specified index to exist, expands wildcards to both open and closed
		* indices and allows that no indices are resolved from wildcard expressions (not returning an error).

Conversation

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

Uh oh!

Uh oh!

elasticmachine commented Apr 24, 2020

Uh oh!

danhermann left a comment

Choose a reason for hiding this comment

Uh oh!

danhermann Apr 24, 2020

Choose a reason for hiding this comment

Uh oh!

henningandersen commented Apr 27, 2020

Uh oh!

martijnvg commented Apr 28, 2020

Uh oh!

henningandersen commented Apr 30, 2020

Uh oh!

martijnvg commented Apr 30, 2020

Uh oh!

martijnvg commented May 8, 2020

Uh oh!

henningandersen left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

martijnvg commented Apr 24, 2020 •

edited

Loading