[Fleet] Update data streams mappings directly instead of against backing indices #89660

jen-huang · 2021-01-29T01:03:42Z

Summary

Resolves #89436. This fixes the concurrent rollover errors when upgrading a package that 1) has incompatible mappings and 2) already has data streams created with different namespaces.

In the case when we detect incompatible mappings, we trigger a rollover on the data stream instead. When this code was written originally in #69180, Elasticsearch did not support updating mappings on the data stream directly, so we had to workaround it by querying for the backing indices. This caused problems when there are multiple namespaces.

ES added support for updating data stream mappings directly in elastic/elasticsearch#58231 (corresponding docs).

This PR:

Returns the data stream name to work with, instead of the last backing index name (aka the write index): LOC
Sends the mapping update attempt against the data stream with write_index_only parameter to protect data in backing indices: LOC
Removes the subsequent query to solve for the data stream name before calling rollover: LOC
Changes variable and method names to reflect working with a data stream, instead of indices (hence the line changes 😉 )

The corresponding API integration test suite was updated to perform against multiple data streams with multiple namespaces.

… indices, update integration tests to test with multiple namespaces

elasticmachine · 2021-01-29T01:03:45Z

Pinging @elastic/fleet (Feature:EPM)

elasticmachine · 2021-01-29T01:03:45Z

Pinging @elastic/ingest-management (Team:Ingest Management)

…llovers

neptunian · 2021-01-29T14:53:24Z

Looks great! This really simplifies things. Though I'd like to make sure, is there any concern about updating all the backing indices of the datastream? The docs say "Changing the mapping of an existing field could invalidate any data that’s already indexed". Is that expected? There is a write_index_only parameter option now so we wouldn't need to query for indices in this case either. CC @ruflin

jen-huang · 2021-01-29T17:37:57Z

@neptunian Thanks for pointing that out! I'm not sure about what the consequences of updating all backing indices mappings are, but it sounds like setting write_index_only would be a good idea to maintain parity with the previous code (where we look for the last backing index to update against). I'd like to hear @ruflin's opinion too, but I'll update the PR with that flag in the meantime.

…llovers

jfsiii · 2021-01-31T15:21:07Z

x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.ts

-    return updateExistingIndex({ indexName, callCluster, indexTemplate });
-  });
-  await Promise.all(updateIndexPromises);
+  const updatedataStreamPromises = indexNameWithTemplates.map(


This should be moved inside the Promise.all. If updateExistingDataStream throws/rejects before the await Promise.all line we'll crash due to UnhandledPromiseRejection

@jfsiii Can you elaborate or provide revised code? updatedataStreamPromises is passed to the Promise.all and updateExistingDataStream is async?

Also, I didn't change the logic here, just renaming. Due to being this a priority fix, I might merge this and follow up on code improvements based on your feedback afterwards.

ruflin · 2021-02-01T07:18:06Z

We should not try to update all backing indices but only the one we currently write to. Otherwise it would update many indices where the new mapping does not matter and in addition it would slow down. Also it would probably trigger a rollover if just a single old index does not match the mapping, so rollover from there forward would always happen. write_index_only sounds like the right flag.

ruflin · 2021-02-01T09:14:51Z

@EricDavisX It would be great if we could add a test case to our e2e using multiple namespaces to uncover this issue here.

jen-huang · 2021-02-01T15:18:53Z

@elasticmachine merge upstream

kibanamachine · 2021-02-01T17:18:26Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 2687f79

Metrics [docs]

✅ unchanged

History

💚 Build #102254 succeeded 8b24a55
💚 Build #102012 succeeded bba3887

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…ing indices (elastic#89660) * Update data streams mappings directly instead of querying for backing indices, update integration tests to test with multiple namespaces * Add flag to only update mappings of the current write index Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

…ing indices (elastic#89660) * Update data streams mappings directly instead of querying for backing indices, update integration tests to test with multiple namespaces * Add flag to only update mappings of the current write index Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.ts # x-pack/test/fleet_api_integration/apis/epm/data_stream.ts

…ing indices (#89660) (#89892) * Update data streams mappings directly instead of querying for backing indices, update integration tests to test with multiple namespaces * Add flag to only update mappings of the current write index Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

…ing indices (#89660) (#89891) * Update data streams mappings directly instead of querying for backing indices, update integration tests to test with multiple namespaces * Add flag to only update mappings of the current write index Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

…ing indices (#89660) (#89896) * Update data streams mappings directly instead of querying for backing indices, update integration tests to test with multiple namespaces * Add flag to only update mappings of the current write index Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.ts # x-pack/test/fleet_api_integration/apis/epm/data_stream.ts

Update data streams mappings directly instead of querying for backing…

82938aa

… indices, update integration tests to test with multiple namespaces

jen-huang added bug Fixes for quality problems that affect the customer experience v8.0.0 Feature:EPM Fleet team's Elastic Package Manager (aka Integrations) project Team:Fleet Team label for Observability Data Collection Fleet team v7.12.0 v7.11.1 labels Jan 29, 2021

jen-huang requested a review from a team January 29, 2021 01:03

jen-huang self-assigned this Jan 29, 2021

jen-huang added release_note:skip Skip the PR/issue when compiling release notes release_note:fix v7.10.3 v7.11.0 and removed release_note:skip Skip the PR/issue when compiling release notes v7.11.1 labels Jan 29, 2021

jen-huang requested a review from neptunian January 29, 2021 01:07

Merge remote-tracking branch 'upstream/master' into fix/concurrent-ro…

bba3887

…llovers

jen-huang added 2 commits January 29, 2021 09:53

Add flag to only update mappings of the current write index

dbd246b

Merge remote-tracking branch 'upstream/master' into fix/concurrent-ro…

8b24a55

…llovers

botelastic bot added the Team:Ingest Management label Jan 29, 2021

jen-huang removed the Team:Ingest Management label Jan 29, 2021

neptunian approved these changes Jan 29, 2021

View reviewed changes

botelastic bot added the Team:Ingest Management label Jan 29, 2021

ph removed the Team:Ingest Management label Jan 29, 2021

jfsiii reviewed Jan 31, 2021

View reviewed changes

Merge branch 'master' into fix/concurrent-rollovers

2687f79

jen-huang merged commit cb16a5c into elastic:master Feb 1, 2021

jen-huang deleted the fix/concurrent-rollovers branch February 1, 2021 17:24

This was referenced Feb 1, 2021

[7.x] [Fleet] Update data streams mappings directly instead of against backing indices (#89660) #89891

Merged

[7.11] [Fleet] Update data streams mappings directly instead of against backing indices (#89660) #89892

Merged

jen-huang mentioned this pull request Feb 1, 2021

[7.10] [Fleet] Update data streams mappings directly instead of against backing indices (#89660) #89896

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fleet] Update data streams mappings directly instead of against backing indices #89660

[Fleet] Update data streams mappings directly instead of against backing indices #89660

jen-huang commented Jan 29, 2021 •

edited

Loading

elasticmachine commented Jan 29, 2021

elasticmachine commented Jan 29, 2021

neptunian commented Jan 29, 2021 •

edited

Loading

jen-huang commented Jan 29, 2021

jfsiii Jan 31, 2021

jen-huang Feb 1, 2021

ruflin commented Feb 1, 2021

ruflin commented Feb 1, 2021

jen-huang commented Feb 1, 2021

kibanamachine commented Feb 1, 2021

[Fleet] Update data streams mappings directly instead of against backing indices #89660

[Fleet] Update data streams mappings directly instead of against backing indices #89660

Conversation

jen-huang commented Jan 29, 2021 • edited Loading

Summary

elasticmachine commented Jan 29, 2021

elasticmachine commented Jan 29, 2021

neptunian commented Jan 29, 2021 • edited Loading

jen-huang commented Jan 29, 2021

jfsiii Jan 31, 2021

Choose a reason for hiding this comment

jen-huang Feb 1, 2021

Choose a reason for hiding this comment

ruflin commented Feb 1, 2021

ruflin commented Feb 1, 2021

jen-huang commented Feb 1, 2021

kibanamachine commented Feb 1, 2021

💚 Build Succeeded

Metrics [docs]

History

jen-huang commented Jan 29, 2021 •

edited

Loading

neptunian commented Jan 29, 2021 •

edited

Loading