Log a warning when documents of unknown types are detected during migration #105213

joshdover · 2021-07-12T12:55:20Z

Summary

This updates our handling of unknown types during Saved Object migrations for the 7.x series. See the rationale and decision in #104690 (comment). The following changes were required to support this:

CHECK_UNKNOWN_DOCUMENTS step will now only log a warning when unknown documents are detected with instructions about how to remove them
All documents of unknown types are excluded from the client-side transformations. This mitigates the issue where documents of unknown types that contain the migrationVersion field would fail the migration. Note that this will need to be changed in order to support the core migrations for ID rewriting in 8.0.

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple nodes—undefined behavior when Kibana nodes are configured with different plugins enabled or installed	Low	Medium	This has never been a supported use case and this assumption is relied upon in this change. Upgrade with different configurations can lead to data loss or corruption. Can be corrected by rolling back to the previous `.kibana` index and retrying the upgrade.

For maintainers

This was checked for breaking API changes and was labeled appropriately

src/core/server/saved_objects/migrations/core/migrate_raw_docs.ts

joshdover · 2021-07-12T13:15:32Z

src/core/server/saved_objects/migrationsv2/model/model.ts

-          ...stateP,
-          controlState: 'FATAL',
-          reason: extractUnknownDocFailureReason(res.left.unknownDocs, stateP.sourceIndex.value),
+          ...nextState,


It's debatable whether or not this should be a left or a right result. I opted to keep it left since we just going to change this again to a failure case in the master branch for 8.0. We also don't have an analog for isRightTypeof and it didn't seem worth adding for just this.

I would say right. we don't need a isRightTypeof, as in right case, we would just returns the unknownDocs array in the right return of checkForUnknownDocs (and just append the log if unknownDocs.length>0). But that's not that important.

FWIW, when I was working with Rudolf I understood that left results were those that ultimately led to an error state (i.e. migrations would fail). right results are all those that carry on with the 'happy path' through the migration algorithm. That being said, I also got the feeling were were trying to avoid error handling in the control flow which is kinda what we end up doing in a few cases.

Thanks for the feedback, pushed a commit to switch to Either.right 👍

src/core/server/saved_objects/migrations/core/migrate_raw_docs.test.ts

elasticmachine · 2021-07-12T13:54:28Z

Pinging @elastic/kibana-core (Team:Core)

src/core/server/saved_objects/migrations/core/migrate_raw_docs.ts

pgayvallet · 2021-07-12T20:25:09Z

src/core/server/saved_objects/migrationsv2/model/model.ts

-          ...stateP,
-          controlState: 'FATAL',
-          reason: extractUnknownDocFailureReason(res.left.unknownDocs, stateP.sourceIndex.value),
+          ...nextState,


I would say right. we don't need a isRightTypeof, as in right case, we would just returns the unknownDocs array in the right return of checkForUnknownDocs (and just append the log if unknownDocs.length>0). But that's not that important.

TinaHeiligers

LGTM

TinaHeiligers · 2021-07-12T23:03:33Z

src/core/server/saved_objects/migrationsv2/model/model.ts

-          ...stateP,
-          controlState: 'FATAL',
-          reason: extractUnknownDocFailureReason(res.left.unknownDocs, stateP.sourceIndex.value),
+          ...nextState,


FWIW, when I was working with Rudolf I understood that left results were those that ultimately led to an error state (i.e. migrations would fail). right results are all those that carry on with the 'happy path' through the migration algorithm. That being said, I also got the feeling were were trying to avoid error handling in the control flow which is kinda what we end up doing in a few cases.

…-unknown-types

kibanamachine · 2021-07-13T10:35:55Z

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

💚 Build #137813 succeeded d71d94f

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

…ration (elastic#105213)

…-png-pdf-report-type * 'master' of github.com:elastic/kibana: (292 commits) bring back KQL autocomplete in timeline + fix last updated (elastic#105380) [Maps] Change TOC pop-up wording to reflect filter change, not search bar change (elastic#105163) Updating urls to upstream elastic repo (elastic#105250) [Maps] Move new vector layer wizard card down (elastic#104797) Exclude registering the cases feature if not enabled (elastic#105292) [Uptime] Alerts - Monitor status alert - check monitor status by monitor.timespan (elastic#104541) updated UI copy (elastic#105184) Log a warning when documents of unknown types are detected during migration (elastic#105213) [Logs UI] Register log threshold rule as lifecycle rule (elastic#104341) [Ingest pipelines] add network direction processor (elastic#103436) [Console] Autocomplete definitions (manual backport) (elastic#105086) [Security Solution] User can make Exceptions for Memory protection alerts (elastic#102196) [Lens] Formula: add validation for multiple field/metrics (elastic#104092) Removing async from file upload and data visualizer plugins start lifecycle (elastic#105197) Fix error when validating the form with non blocking validations (elastic#103629) [ML] Fix "View by" swim lane with applied filter and sorting by score (elastic#105217) Update dependency @elastic/charts to v32 (elastic#104625) [CTI] shortens large numbers on Dashboard Link Panel (elastic#105269) [Security Solution][Endpoint][Host Isolation] Fixes bug to remove excess host metadata status toasts on non user initiated errors (elastic#105331) [Cases] Fix pushing alerts count on every push to external service (elastic#105030) ... # Conflicts: # x-pack/plugins/reporting/common/types.ts

…ing migration (#105213) (#105415) * Log a warning when documents of unknown types are detected during migration (#105213) * Parameterize test for kibana version

…ng migration (#105213) (#105414) * Log a warning when documents of unknown types are detected during migration (#105213) * Parameterize test for kibana version

…118300) * manually revert #105213 * enabled xpack to register types for some IT tests * fiz doc size Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

…lastic#118300) * manually revert elastic#105213 * enabled xpack to register types for some IT tests * fiz doc size Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

…118300) (#118690) * manually revert #105213 * enabled xpack to register types for some IT tests * fiz doc size Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Pierre Gayvallet <pierre.gayvallet@elastic.co>

…lastic#118300) * manually revert elastic#105213 * enabled xpack to register types for some IT tests * fiz doc size Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

…118300) * manually revert #105213 * enabled xpack to register types for some IT tests * fiz doc size Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

…lastic#118300) * manually revert elastic#105213 * enabled xpack to register types for some IT tests * fiz doc size Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

joshdover added 2 commits July 12, 2021 14:22

Make CHECK_UNKONWN_DOCUMENTS step log warning instead of failing

4aa3f3f

Skip transformations on unknown types

d71d94f

joshdover commented Jul 12, 2021

View reviewed changes

src/core/server/saved_objects/migrations/core/migrate_raw_docs.ts Show resolved Hide resolved

joshdover commented Jul 12, 2021

View reviewed changes

src/core/server/saved_objects/migrations/core/migrate_raw_docs.test.ts Show resolved Hide resolved

joshdover added release_note:skip Skip the PR/issue when compiling release notes v7.14.0 v7.15.0 v8.0.0 labels Jul 12, 2021

joshdover changed the title ~~Log a warning on documents of unknown types are detected during migration~~ Log a warning when documents of unknown types are detected during migration Jul 12, 2021

joshdover marked this pull request as ready for review July 12, 2021 13:54

joshdover requested a review from a team as a code owner July 12, 2021 13:54

joshdover added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Jul 12, 2021

joshdover mentioned this pull request Jul 12, 2021

[SO Migration] increasing performance by getting rid of the temporary index #104080

Closed

pgayvallet approved these changes Jul 12, 2021

View reviewed changes

TinaHeiligers approved these changes Jul 12, 2021

View reviewed changes

joshdover added 2 commits July 13, 2021 10:27

Switch to Either.right for unknown doc check

c5fb8f5

Merge remote-tracking branch 'upstream/master' into migrations/ignore…

5337e8f

…-unknown-types

joshdover enabled auto-merge (squash) July 13, 2021 08:28

pgayvallet mentioned this pull request Jul 13, 2021

Kibana migration to 7.13.2 issue #103111

Closed

joshdover merged commit 6b26949 into elastic:master Jul 13, 2021

This was referenced Jul 13, 2021

[7.x] Log a warning when documents of unknown types are detected during migration (#105213) #105414

Merged

[7.14] Log a warning when documents of unknown types are detected during migration (#105213) #105415

Merged

joshdover added a commit to joshdover/kibana that referenced this pull request Jul 13, 2021

Log a warning when documents of unknown types are detected during mig…

42bb17f

…ration (elastic#105213)

joshdover mentioned this pull request Jul 13, 2021

Parameterize migration test for kibana version #105417

Merged

1 task

pgayvallet mentioned this pull request Nov 9, 2021

Refuse to start with unknown saved object types in 8.0 #107678

Closed

pgayvallet added a commit to pgayvallet/kibana that referenced this pull request Nov 11, 2021

manually revert elastic#105213

686e66e

pgayvallet mentioned this pull request Nov 11, 2021

[SO migration] fail the migration if unknown types are encountered #118300

Merged

1 task

rudolf mentioned this pull request Jun 21, 2022

don't merge mappings from source index #134809

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Log a warning when documents of unknown types are detected during migration #105213

Log a warning when documents of unknown types are detected during migration #105213

joshdover commented Jul 12, 2021 •

edited

Loading

joshdover Jul 12, 2021 •

edited

Loading

pgayvallet Jul 12, 2021 •

edited

Loading

TinaHeiligers Jul 12, 2021

joshdover Jul 13, 2021

elasticmachine commented Jul 12, 2021

pgayvallet Jul 12, 2021 •

edited

Loading

TinaHeiligers left a comment

TinaHeiligers Jul 12, 2021

kibanamachine commented Jul 13, 2021

Log a warning when documents of unknown types are detected during migration #105213

Log a warning when documents of unknown types are detected during migration #105213

Conversation

joshdover commented Jul 12, 2021 • edited Loading

Summary

Checklist

Risk Matrix

For maintainers

joshdover Jul 12, 2021 • edited Loading

Choose a reason for hiding this comment

pgayvallet Jul 12, 2021 • edited Loading

Choose a reason for hiding this comment

TinaHeiligers Jul 12, 2021

Choose a reason for hiding this comment

joshdover Jul 13, 2021

Choose a reason for hiding this comment

elasticmachine commented Jul 12, 2021

pgayvallet Jul 12, 2021 • edited Loading

Choose a reason for hiding this comment

TinaHeiligers left a comment

Choose a reason for hiding this comment

TinaHeiligers Jul 12, 2021

Choose a reason for hiding this comment

kibanamachine commented Jul 13, 2021

💚 Build Succeeded

Metrics [docs]

History

joshdover commented Jul 12, 2021 •

edited

Loading

joshdover Jul 12, 2021 •

edited

Loading

pgayvallet Jul 12, 2021 •

edited

Loading

pgayvallet Jul 12, 2021 •

edited

Loading