Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Do not skip UPDATE_TARGET_MAPPINGS if upgrading to a newer stack version #147503

Merged
merged 1 commit into from
Dec 14, 2022
Merged

Do not skip UPDATE_TARGET_MAPPINGS if upgrading to a newer stack version #147503

merged 1 commit into from
Dec 14, 2022

Conversation

gsoldevila
Copy link
Contributor

@gsoldevila gsoldevila commented Dec 14, 2022 •
edited by kibanamachine
Loading

Fixes #147450

@gsoldevila gsoldevila added bug Fixes for quality problems that affect the customer experience Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc release_note:skip Skip the PR/issue when compiling release notes Feature:Migrations v8.6.0 v8.7.0 labels Dec 14, 2022
@gsoldevila gsoldevila requested a review from a team as a code owner December 14, 2022 11:23
afharo
afharo reviewed Dec 14, 2022
View reviewed changes
Copy link
Member

@afharo afharo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes LGTM! However, I'd defer the approval to someone else with more experience in SO migrations.

@afharo afharo requested a review from a team December 14, 2022 11:55
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/core-test-helpers-kbn-server 46 48 +2
Unknown metric groups

API count

id before after diff
@kbn/core-test-helpers-kbn-server 52 54 +2

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 60 66 +6
osquery 109 115 +6
securitySolution 445 451 +6
total +20

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 69 75 +6
osquery 110 117 +7
securitySolution 521 527 +6
total +21

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

@gsoldevila gsoldevila added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) v8.6.1 labels Dec 14, 2022
@gsoldevila gsoldevila merged commit 4f72a2e into elastic:main Dec 14, 2022
@gsoldevila gsoldevila mentioned this pull request Dec 14, 2022
Closed
@gsoldevila
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
✅ 8.6

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

gsoldevila added a commit to gsoldevila/kibana that referenced this pull request Dec 14, 2022
@gsoldevila
Do not skip UPDATE_TARGET_MAPPINGS if upgrading to a newer stack vers…
fc36556 
…ion (elastic#147503)

Fixes elastic#147450

(cherry picked from commit 4f72a2e)
@kibanamachine kibanamachine mentioned this pull request Dec 14, 2022
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 14, 2022
@gsoldevila
Do not skip UPDATE_TARGET_MAPPINGS if upgrading to a newer stack vers…
6e07c9a 
…ion (elastic#147503)

Fixes elastic#147450

(cherry picked from commit 4f72a2e)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.6

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Dec 14, 2022
@kibanamachine @gsoldevila
[8.6] Do not skip UPDATE_TARGET_MAPPINGS if upgrading to a newer stac…
c9ca848 
…k version (#147503) (#147522)

# Backport

This will backport the following commits from `main` to `8.6`:
- [Do not skip UPDATE_TARGET_MAPPINGS if upgrading to a newer stack
version (#147503)](#147503)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Gerard
Soldevila","email":"[email protected]"},"sourceCommit":{"committedDate":"2022-12-14T13:39:47Z","message":"Do
not skip UPDATE_TARGET_MAPPINGS if upgrading to a newer stack version
(#147503)\n\nFixes
https://github.com/elastic/kibana/issues/147450","sha":"4f72a2eb5beded9a3086de7766021d9b65a197f3","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","Team:Core","release_note:skip","Feature:Migrations","backport:prev-minor","v8.6.0","v8.7.0","v8.6.1"],"number":147503,"url":"https://github.com/elastic/kibana/pull/147503","mergeCommit":{"message":"Do
not skip UPDATE_TARGET_MAPPINGS if upgrading to a newer stack version
(#147503)\n\nFixes
https://github.com/elastic/kibana/issues/147450","sha":"4f72a2eb5beded9a3086de7766021d9b65a197f3"}},"sourceBranch":"main","suggestedTargetBranches":["8.6"],"targetPullRequestStates":[{"branch":"8.6","label":"v8.6.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/147503","number":147503,"mergeCommit":{"message":"Do
not skip UPDATE_TARGET_MAPPINGS if upgrading to a newer stack version
(#147503)\n\nFixes
https://github.com/elastic/kibana/issues/147450","sha":"4f72a2eb5beded9a3086de7766021d9b65a197f3"}}]}]
BACKPORT-->

Co-authored-by: Gerard Soldevila <[email protected]>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Dec 14, 2022
@jloleysens
Merge branch 'main' into only-migrate-if-needed-KBNA-4488
60d2987 
* main: (21 commits)
  [Profiling] Remove link to 'Other' bucket (elastic#147523)
  [Synthetics UI] Add missing configuration options to the add/edit monitor forms (elastic#147265)
  [DOCS] Updates what's new pages (elastic#147483)
  [Fleet][Endpoint][RBAC V2] Update fleet router and config to allow API access via RBAC controls (elastic#145361)
  [Guided onboarding] Update guide IDs (elastic#147348)
  [Synthetics] Add synthetics settings alerting default (elastic#147339)
  [Security Solution][Endpoint] Fix Policy form being displayed as Read Only when displayed in Fleet pages (elastic#147212)
  [Cases] Save draft user comment (elastic#146327)
  [API Docs] Fix `--plugin` filter (elastic#147500)
  [Fleet] added a logic to use `destinationId` when tagging imported SOs (elastic#147439)
  Do not skip UPDATE_TARGET_MAPPINGS if upgrading to a newer stack version (elastic#147503)
  [Discover] Validate if Data View time field exists on Alert creation / editing (elastic#146324)
  [Discover] Fix Discover navigation from Lens embeddable (elastic#147000)
  Allow users to Update API Keys (elastic#146237)
  Update dependency xstate to ^4.35.0 (main) (elastic#147463)
  [Behavioral Analytics] Remove feature flag to hide functionality (elastic#147429)
  [Fleet] Add agent policy `inactivity_timeout`experimental setting (elastic#147432)
  [APM] Switching service groups from grid to flex layout (elastic#147448)
  [Fleet] Add missing endpoints to openApi specs (elastic#147452)
  [AO] Allow providing custom time range for Alert Summary Widget (elastic#147253)
  ...
@jloleysens jloleysens mentioned this pull request Dec 15, 2022
Merged
1 task
nreese pushed a commit to nreese/kibana that referenced this pull request Dec 16, 2022
@gsoldevila @nreese
Do not skip UPDATE_TARGET_MAPPINGS if upgrading to a newer stack vers…
8f8d6e2 
…ion (elastic#147503)

Fixes elastic#147450
gsoldevila added a commit that referenced this pull request Dec 23, 2022
@jloleysens @kibanamachine @gsoldevila
[Migrations] Only reindex indices if needed (#147371)
bcb8a13 
Fixes #124946

## Summary

Takes a step toward optimising our migration paths by only reindexing
(an expensive operation) when needed by checking whether the current SO
mappings have "changed".

By "changed" we mean that we have detected a new md5 checksum of any
registered saved object type relative to the hashes we have stored.

## How to test

These changes are constrained to the `model.ts`, a test was added for
correctly detecting that mappings are the same during the `INIT` phase
to send us down the correct migration path.

Additionally, we have a new Jest integration test `skip_reindex.test.ts`
that runs Kibana and inspects the logs for the expected model
transitions.

Everything else should remain the same.

## Happy path for skipping reindex

```
RUN INIT

IF !versionMigrationIsComplete AND
   !kibana indexBelongsToLaterVersion AND
   !waitForMigrationCompletion AND
   mappingsAreUnchanged
THEN
  the migration operations must target the existing .kibana_x.y.z_001 index

RUN PREPARE_COMPATIBLE_MIGRATION (new state)
    we remove old version aliases (prevent old reads/writes), and set the current version alias (prevent old migrations)

SKIP LEGACY_SET_WRITE_BLOCK
SKIP ...
SKIP SET_SOURCE_WRITE_BLOCK 
SKIP ...
SKIP REFRESH_TARGET

RUN OUTDATED_DOCUMENTS_SEARCH_OPEN_PIT
...
RUN MARK_VERSION_INDEX_READY
DONE
```

## Notes

* This optimisation will only be triggered when there are no mappings
changes AND we are upgrading to a new version. This does not cover all
cases. In future we will make this more sophisticated by checking for
incompatible changes to mappings and only reindexing when those occur.

## Related

* #147503

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Gerard Soldevila <[email protected]>
@rudolf rudolf added the Epic:ScaleMigrations Scale upgrade migrations to millions of saved objects label Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@afharo afharo afharo left review comments

@rudolf rudolf rudolf approved these changes

Assignees
No one assigned
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) bug Fixes for quality problems that affect the customer experience Epic:ScaleMigrations Scale upgrade migrations to millions of saved objects Feature:Migrations release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.6.0 v8.6.1 v8.7.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Migration logic doesn't set the right mappings when upgrading to a newer version if the mappings don't change
5 participants
@gsoldevila @kibana-ci @kibanamachine @rudolf @afharo