Ingest ingest then create index #39607

jakelandis · 2019-03-02T23:25:14Z

Execute ingest node pipeline before creating the index

Prior to this commit (and after 6.5.0), if an ingest node changes
the _index in a pipeline, the original target index would be created.
For daily indexes this could create an extra, empty index per day.

This commit changes the TransportBulkAction to execute the ingest node
pipeline before attempting to create the index. This ensures that the
only index created is the original or one set by the ingest node pipeline.
This was the execution order prior to 6.5.0 (#32786).

The execution order was changed in 6.5 to better support default pipelines.
Specifically the execution order was changed to be able to read the settings
from the index meta data. This commit also includes a change in logic such
that if the target index does not exist when ingest node pipeline runs, it
will now pull the default pipeline (if one exists) from the settings of the
best matched of the index template.

Relates #32786
Relates #32758
Closes #36545

There is some history here...

Originally ingest node executed before any creation of indices. We introduced default pipelines and the discovered #32758 (the first doc in an index failed to get run through the pipeline). So we fixed that via #32758 (changes to create the index first). However, that fix introduced #36545 (comment) (note - the title is misleading) where now if the pipeline re-directs the _index it still creates the original _index resulting in an empty index. This is particularly problematic for daily indices that always re-directed, since it would result in an empty index per day. This (re-directing daily indices) is also how upgrading monitoring templates work, and thus this PR is a blocker to #39336.

The fix here is to go back to the original order where ingest node executes before creation of indexes, but solve #32758 differently. Instead of creating the index and always reading the index meta data, if the index does not exist we can now do the index template matching to look for the default pipeline.

elasticmachine · 2019-03-02T23:25:15Z

Pinging @elastic/es-core-features

jakelandis · 2019-03-02T23:28:27Z

modules/ingest-common/src/test/resources/rest-api-spec/test/ingest/220_drop_processor.yml

@@ -91,4 +91,4 @@ teardown:
    get:
      index: test
      id: 3
- match: { found: false }


This is removed because with this change the test index is no longer created since the index creation only happens after ingest (and this ingest drops the index request, thus no index ).

jakelandis · 2019-03-02T23:32:48Z

server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java

+                        if (IngestService.NOOP_PIPELINE_NAME.equals(defaultPipeline) == false) {
+                            hasIndexRequestsWithPipelines = true;
+                        }
+                    } else if (indexRequest.index() != null) {


Moving the ingest node execution before the create index is the primary change of this PR.

However, to allow the prevent re-introducing #32758, we need to look up the default pipeline from the matched templates which is what this code block does.

jakelandis · 2019-03-02T23:34:29Z

server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIngestTests.java

@@ -460,7 +462,7 @@ public void testUseDefaultPipelineWithBulkUpsert() throws Exception {
        verifyZeroInteractions(transportService);
    }

-    public void testCreateIndexBeforeRunPipeline() throws Exception {
+    public void testDoExecuteCalledTwiceCorrectly() throws Exception {


This test is valid, but it's name was slightly misleading (it still passed with my change to run pipeline before the create index)

jakelandis · 2019-03-04T14:05:56Z

docs/reference/ingest/ingest-node.asciidoc

--------------------------------------------------
-// TESTRESPONSE
-////
-


these were removed because now that the ingest happens before index creation, if the index doesn't exist yet, and we only drop a document, the index never exists, and thus the response is different.

martijnvg · 2019-03-05T15:17:58Z

@jakelandis I checked this out and the approach in the pr looks good to me.

jakelandis · 2019-03-05T15:51:22Z

@martijnvg @original-brownbear thanks for taking a look.

I forgot to pull the PR out of draft mode, could I get a review approval or LGTM (assuming this is good to go).

original-brownbear · 2019-03-05T16:00:43Z

@jakelandis giving it a thorough review now then, won't take long :)

original-brownbear

Only one problem, otherwise LGTM

server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java

martijnvg

👍

Prior to this commit (and after 6.5.0), if an ingest node changes the _index in a pipeline, the original target index would be created. For daily indexes this could create an extra, empty index per day. This commit changes the TransportBulkAction to execute the ingest node pipeline before attempting to create the index. This ensures that the only index created is the original or one set by the ingest node pipeline. This was the execution order prior to 6.5.0 (elastic#32786). The execution order was changed in 6.5 to better support default pipelines. Specifically the execution order was changed to be able to read the settings from the index meta data. This commit also includes a change in logic such that if the target index does not exist when ingest node pipeline runs, it will now pull the default pipeline (if one exists) from the settings of the best matched of the index template. Relates elastic#32786 Relates elastic#32758 Closes elastic#36545

Prior to this commit (and after 6.5.0), if an ingest node changes the _index in a pipeline, the original target index would be created. For daily indexes this could create an extra, empty index per day. This commit changes the TransportBulkAction to execute the ingest node pipeline before attempting to create the index. This ensures that the only index created is the original or one set by the ingest node pipeline. This was the execution order prior to 6.5.0 (#32786). The execution order was changed in 6.5 to better support default pipelines. Specifically the execution order was changed to be able to read the settings from the index meta data. This commit also includes a change in logic such that if the target index does not exist when ingest node pipeline runs, it will now pull the default pipeline (if one exists) from the settings of the best matched of the index template. Relates #32786 Relates #32758 Closes #36545

jakelandis added 2 commits March 2, 2019 16:50

main fix and tests, still need itests

12aad7e

itests

44b4ef1

jakelandis added WIP :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP v7.0.0 v8.0.0 v7.2.0 labels Mar 2, 2019

jakelandis commented Mar 2, 2019

View reviewed changes

fix doc tests

f64fdc3

jakelandis commented Mar 4, 2019

View reviewed changes

jakelandis removed the WIP label Mar 4, 2019

jakelandis requested review from original-brownbear and martijnvg March 4, 2019 14:23

jakelandis marked this pull request as ready for review March 5, 2019 15:45

original-brownbear requested changes Mar 5, 2019

View reviewed changes

server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java Outdated Show resolved Hide resolved

jakelandis added 2 commits March 5, 2019 15:25

properly handle the default setting when template matching

efdaf53

Merge branch 'master' into ingest_ingest_then_create_index

18a30a2

jakelandis mentioned this pull request Mar 6, 2019

Remove types from index templates for x-pack features #38637

Closed

4 tasks

martijnvg approved these changes Mar 6, 2019

View reviewed changes

fix loop

80d71b4

original-brownbear approved these changes Mar 6, 2019

View reviewed changes

Merge branch 'master' into ingest_ingest_then_create_index

c528171

jakelandis merged commit 66ec358 into elastic:master Mar 6, 2019

jakelandis added the backport pending label Mar 6, 2019

jakelandis added the v6.7.0 label Mar 7, 2019

jakelandis removed the backport pending label Mar 8, 2019

danielmitterdorfer added the >bug label Mar 14, 2019

jakelandis added v7.0.0-rc2 and removed v7.0.0 labels Apr 3, 2019

jakelandis mentioned this pull request May 10, 2019

Honor default_pipeline on scripted _index rewrites #42019

Open

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

Ingest ingest then create index #39607

Ingest ingest then create index #39607

jakelandis commented Mar 2, 2019 •

edited

Loading

elasticmachine commented Mar 2, 2019

jakelandis Mar 2, 2019 •

edited

Loading

jakelandis Mar 2, 2019

jakelandis Mar 2, 2019 •

edited

Loading

jakelandis Mar 4, 2019 •

edited

Loading

martijnvg commented Mar 5, 2019

jakelandis commented Mar 5, 2019

original-brownbear commented Mar 5, 2019

original-brownbear left a comment

martijnvg left a comment

Ingest ingest then create index #39607

Ingest ingest then create index #39607

Conversation

jakelandis commented Mar 2, 2019 • edited Loading

elasticmachine commented Mar 2, 2019

jakelandis Mar 2, 2019 • edited Loading

Choose a reason for hiding this comment

jakelandis Mar 2, 2019

Choose a reason for hiding this comment

jakelandis Mar 2, 2019 • edited Loading

Choose a reason for hiding this comment

jakelandis Mar 4, 2019 • edited Loading

Choose a reason for hiding this comment

martijnvg commented Mar 5, 2019

jakelandis commented Mar 5, 2019

original-brownbear commented Mar 5, 2019

original-brownbear left a comment

Choose a reason for hiding this comment

martijnvg left a comment

Choose a reason for hiding this comment

jakelandis commented Mar 2, 2019 •

edited

Loading

jakelandis Mar 2, 2019 •

edited

Loading

jakelandis Mar 2, 2019 •

edited

Loading

jakelandis Mar 4, 2019 •

edited

Loading