[ML] Add indices_options to datafeed config and update #52793

benwtrent · 2020-02-25T19:42:50Z

This adds a new configurable field called indices_options. This allows users to create or update the indices_options used when a datafeed reads from an index.

This is necessary for the following use cases:

Reading from frozen indices
Allowing certain indices in multiple index patterns to not exist yet

These index options are available on datafeed creation and update. Users may specify them as URL parameters or within the configuration object.

closes #48056

elasticmachine · 2020-02-25T19:42:53Z

Pinging @elastic/ml-core (:ml)

dimitris-athanasiou

Looks good! My only confusion has to do with handling default options when it is not set. In particular, it seems we used to have 2 different defaults in different places: IndicesOptions.lenientExpandOpen() and SearchRequest.DEFAULT_INDICES_OPTIONS, which is IndicesOptions.strictExpandOpenAndForbidClosedIgnoreThrottled().

With this change defaults will remain the same when there are no options set. But if a user sets options explicitly, we use SearchRequest.DEFAULT_INDICES_OPTIONS as the default options. This is the part that confuses me.

dimitris-athanasiou · 2020-02-26T09:18:50Z

...plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/datafeeds/RestPutDatafeedAction.java

@@ -44,8 +46,15 @@ public String getName() {
    @Override
    protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
        String datafeedId = restRequest.param(DatafeedConfig.ID.getPreferredName());
+        IndicesOptions indicesOptions = null;
+        if (restRequest.hasParam("expand_wildcards")


Could we avoid checking if restRequest has those params? I think it'd work to do IndicesOptions indicesOptions = IndicesOptions.fromRequest(restRequest, SearchRequest.DEFAULT_INDICES_OPTIONS)

Makes sense. DatafeedConfig can make the options non-null and default to SearchRequestt.DEFAULT_INDICES_OPTIONS

dimitris-athanasiou · 2020-02-26T09:20:33Z

...gin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/datafeeds/RestUpdateDatafeedAction.java

@@ -44,8 +46,15 @@ public String getName() {
    @Override
    protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
        String datafeedId = restRequest.param(DatafeedConfig.ID.getPreferredName());
+        IndicesOptions indicesOptions = null;
+        if (restRequest.hasParam("expand_wildcards")


I don't agree. If none are provided, then that means the user doesn't want to update them.

Ah, yes. That's the update. My bad!

droberts195 · 2020-02-26T10:58:11Z

it seems we used to have 2 different defaults in different places: IndicesOptions.lenientExpandOpen() and SearchRequest.DEFAULT_INDICES_OPTIONS, which is IndicesOptions.strictExpandOpenAndForbidClosedIgnoreThrottled().

It seems that this was a bug before, and it didn't really matter in the days before frozen indices but frozen indices mean it could cause weird behaviour.

We discussed this a bit in a different channel and the outcome was:

We should use the same default in both places, and that default should be SearchRequest.DEFAULT_INDICES_OPTIONS
As a result of this, the indicesOptions member of DatafeedConfig can be made @NotNull and defaulted to SearchRequest.DEFAULT_INDICES_OPTIONS at an earlier stage - this will reduce the number of null checks required downstream

We have an extra check for no concrete indices in addition to the indices options check:

elasticsearch/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedNodeSelector.java

Lines 125 to 127 in 7e4c0e6

    
           if (concreteIndices.length == 0) { 
        
               return new AssignmentFailure(reason, true); 
        
           }

Currently that sets the same assignment failure reason as an exception from the indices options check sets, and this could lead to confusion if indices options include allow_no_indices. So the reason returned on line 126 of current master/line 128 of commit 1 on this PR should be made different to the reason returned on line 130 of current master/line 132 of commit 1 on this PR. The current reason is most suited to the if (concreteIndices.length == 0) check and the new reason for the resolver throwing should include the toString() representation of the caught exception so that it reflects the problem given the configured indices options.

droberts195

LGTM

…ed-add-indices_options

dimitris-athanasiou

LGTM2

This adds a new configurable field called `indices_options`. This allows users to create or update the indices_options used when a datafeed reads from an index. This is necessary for the following use cases: - Reading from frozen indices - Allowing certain indices in multiple index patterns to not exist yet These index options are available on datafeed creation and update. Users may specify them as URL parameters or within the configuration object. closes elastic#48056

This adds a new configurable field called `indices_options`. This allows users to create or update the indices_options used when a datafeed reads from an index. This is necessary for the following use cases: - Reading from frozen indices - Allowing certain indices in multiple index patterns to not exist yet These index options are available on datafeed creation and update. Users may specify them as URL parameters or within the configuration object. closes #48056

Re-enables bwc tests and fixes up code after backport of #52793

[ML] Add indices_options to datafeed config

f6db6dc

benwtrent added >enhancement :ml Machine learning v8.0.0 v7.7.0 labels Feb 25, 2020

dimitris-athanasiou reviewed Feb 26, 2020

View reviewed changes

addressing PR comments

2070eb8

benwtrent requested a review from dimitris-athanasiou February 26, 2020 15:36

droberts195 approved these changes Feb 26, 2020

View reviewed changes

benwtrent added 3 commits February 26, 2020 14:13

addressing PR comments

70caa0d

Merge remote-tracking branch 'upstream/master' into feature/ml-datafe…

9f0ef5a

…ed-add-indices_options

reverting bad assertion

1c9cd95

dimitris-athanasiou approved these changes Feb 27, 2020

View reviewed changes

mute for bwc

e21eae7

benwtrent merged commit d7a6333 into elastic:master Feb 27, 2020

benwtrent deleted the feature/ml-datafeed-add-indices_options branch February 27, 2020 17:22

This was referenced Feb 27, 2020

[7.x] [ML] Add indices_options to datafeed config and update (#52793) #52905

Merged

[ML] fixing datafeed bwc tests #52908

Merged

benwtrent added a commit that referenced this pull request Feb 28, 2020

[ML] fixing datafeed bwc tests (#52908)

1d4a9bb

Re-enables bwc tests and fixes up code after backport of #52793

jgowdyelastic mentioned this pull request Mar 3, 2020

[ML] Adding indices_options to datafeed elastic/kibana#59119

Merged

lcawl mentioned this pull request Mar 3, 2020

[DOCS] Adds link in datafeed indices_options #53067

Merged

droberts195 mentioned this pull request Mar 27, 2020

[ML] Use datafeed indices options when doing preview search elastic/kibana#61664

Closed

codebrain mentioned this pull request Apr 1, 2020

7.7.0 meta ticket (Part 2) elastic/elasticsearch-net#4533

Closed

codebrain mentioned this pull request Apr 14, 2020

7.7.0 meta ticket elastic/elasticsearch-net#4525

Closed

38 tasks

droberts195 mentioned this pull request Nov 30, 2020

[Transform] Add support for ignore_throttled #65594

Closed

jakelandis removed the v8.0.0 label Jul 26, 2021

jakelandis added the v8.0.0-alpha1 label Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Add indices_options to datafeed config and update #52793

[ML] Add indices_options to datafeed config and update #52793

benwtrent commented Feb 25, 2020

elasticmachine commented Feb 25, 2020

dimitris-athanasiou left a comment

dimitris-athanasiou Feb 26, 2020

benwtrent Feb 26, 2020

dimitris-athanasiou Feb 26, 2020

benwtrent Feb 26, 2020

dimitris-athanasiou Feb 26, 2020

droberts195 commented Feb 26, 2020

droberts195 left a comment

dimitris-athanasiou left a comment

[ML] Add indices_options to datafeed config and update #52793

[ML] Add indices_options to datafeed config and update #52793

Conversation

benwtrent commented Feb 25, 2020

elasticmachine commented Feb 25, 2020

dimitris-athanasiou left a comment

Choose a reason for hiding this comment

dimitris-athanasiou Feb 26, 2020

Choose a reason for hiding this comment

benwtrent Feb 26, 2020

Choose a reason for hiding this comment

dimitris-athanasiou Feb 26, 2020

Choose a reason for hiding this comment

benwtrent Feb 26, 2020

Choose a reason for hiding this comment

dimitris-athanasiou Feb 26, 2020

Choose a reason for hiding this comment

droberts195 commented Feb 26, 2020

droberts195 left a comment

Choose a reason for hiding this comment

dimitris-athanasiou left a comment

Choose a reason for hiding this comment