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.

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

[ZDT] Implement ModelVersion polling mechanism #154329

Closed

Conversation

jloleysens
Copy link
Contributor

Summary

Close #152809.

To reviewers

  • This PR only adds the implementation for the poller
  • Implementation hidden behind an observable interface exposed as an Observable and a "get current" function
  • Currently it is assumed that we do not need to poll if no-one is observing

@jloleysens jloleysens added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:Saved Objects release_note:skip Skip the PR/issue when compiling release notes Epic:ZDTmigrations Zero downtime migrations v8.8.0 labels Apr 4, 2023
@jloleysens jloleysens self-assigned this Apr 4, 2023
@jloleysens jloleysens requested a review from a team as a code owner April 4, 2023 11:17
@elasticmachine
Copy link
Contributor

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

@jloleysens jloleysens changed the title [ZDT] Implement polling mechanism [ZDT] Implement ModelVersion polling mechanism Apr 4, 2023
Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

LGTM, with a few questions/comments.
We'll also eventually need some sort of documentation on the polling mechanism. A copy-paste of the code comments should be enough for now.


const POLL_INTERVAL_MS = 30_000;
/** TODO: Is this the correct pattern? In this way we will also be matching indices after "the split" */
const KIBANA_SYSTEM_INDICES_PATTERN = '.kibana_*';
Copy link
Contributor

Choose a reason for hiding this comment

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

We could match on a one-of or entries from a collection of patterns. EIther a map or, to keep it simple, an array of patterns.

});
} catch (e) {
if (
e instanceof errors.ResponseError &&
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the risk of an intermittent proxy response? Should we also be checking the headers?

* (1) Run a request against Elasticsearch for Kibana system indices. For a
* class of bad responses will retry up to 5 times with backoff.
* (2) Build a model version map and emit this value
* (3) Wait for `pollInterval` ms before repeating the process.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we poll indefinitely and, if so, is that to address the concern around potential rollbacks?
The original issue suggests only polling until "the index version matches the app version".

@kibana-ci
Copy link
Collaborator

kibana-ci commented Apr 5, 2023

💔 Build Failed

Failed CI Steps

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 432 435 +3

Total ESLint disabled count

id before after diff
securitySolution 512 515 +3

History

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

cc @jloleysens

Comment on lines +42 to +45
return await client.indices.getMapping({
index: KIBANA_SYSTEM_INDICES_PATTERN,
expand_wildcards: 'hidden',
});
Copy link
Member

Choose a reason for hiding this comment

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

I'd use filter_path to only fetch the _meta

@rayafratkina
Copy link
Contributor

Since we are no longer doing this for MVP, we should probably close this

@rudolf
Copy link
Contributor

rudolf commented Sep 14, 2023

No longer needed.

@rudolf rudolf closed this Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic:ZDTmigrations Zero downtime migrations Feature:Saved Objects 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.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SOR: implement polling to retrieve the current model version
8 participants