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

Don't start pollEsNodesVersion unless someone subscribes #56923

Merged
merged 8 commits into from
Feb 28, 2020

Conversation

rudolf
Copy link
Contributor

@rudolf rudolf commented Feb 5, 2020

By not polling until subscribed to, we prevent verbose error logs when
the optimizer is run (which automatically skips migrations).

Summary

Summarize your PR. If it involves visual changes include a screenshot or gif.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@streamich streamich added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Feb 7, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

By not polling until subscribed to, we prevent verbose error logs when
the optimizer is run (which automatically skips migrations).
@rudolf rudolf force-pushed the quiet-version-check-during-optimize branch from 22cbc8b to 52d03e3 Compare February 11, 2020 15:14
@rudolf rudolf marked this pull request as ready for review February 17, 2020 13:30
@rudolf rudolf requested a review from a team as a code owner February 17, 2020 13:30
this.log.error(message);
}
});
}).pipe(shareReplay({ refCount: true, bufferSize: 1 }));
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you use bufferSize: 1 to reduce memory footprint?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The buffer size is the size of the replay subject that shareReplay uses internally. As long as there are subscribers, any new subscribers will get the last 1 values. So as long as we're polling, any new subscribers will immediately get the latest version check results.

When there are no subscribers, refCount drops to 0 and the internal replay subject will unsubscribe from pollEsNodesVersion$. When there's no subscriptions and then we get one new subscriber, that subscriber won't get any buffered/replayed version checks since these could be very old. Only once pollEsNodesVersion$ emits a value again will the first subscriber get a result.

@rudolf rudolf added release_note:skip Skip the PR/issue when compiling release notes v7.6.1 v8.0.0 labels Feb 21, 2020
@rudolf
Copy link
Contributor Author

rudolf commented Feb 24, 2020

@elasticmachine merge upstream

@rudolf rudolf requested a review from mshustov February 24, 2020 11:01
@rudolf
Copy link
Contributor Author

rudolf commented Feb 26, 2020

@elasticmachine merge upstream

@rudolf rudolf removed the v7.6.1 label Feb 26, 2020
@rudolf
Copy link
Contributor Author

rudolf commented Feb 28, 2020

retest

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@rudolf rudolf merged commit 56ba753 into elastic:master Feb 28, 2020
@rudolf rudolf deleted the quiet-version-check-during-optimize branch February 28, 2020 21:33
rudolf added a commit to rudolf/kibana that referenced this pull request Feb 28, 2020
* Don't start pollEsNodesVersion unless someone subscribes

By not polling until subscribed to, we prevent verbose error logs when
the optimizer is run (which automatically skips migrations).

* Test pollEsNodeVersions behaviour

* Cleanup unused code

* PR Feedback

* Make test more stable

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 29, 2020
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

1 similar comment
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

jloleysens added a commit to jloleysens/kibana that referenced this pull request Mar 2, 2020
…dex-server-side

* 'master' of github.com:elastic/kibana: (34 commits)
  [Upgrade Assistant] Remove "boom" from reindex service (elastic#58715)
  [data] Clean up QueryStringInput unit tests (elastic#58704)
  [SIEM] Detection Fix typo in Adobe Hijack Persistence rule (elastic#58804)
  Restores [SIEM][CASE] Init Configure Case Page (elastic#58121) (elastic#58924)
  Skips additional failing Ingest Manager integration tests
  Skips failing Ingest Manager integration tests
  Move dev tools styles to NP (elastic#58855)
  change to have kibana --ssl cli option use more recent certs (elastic#57933)
  disable failing suite (elastic#58942)
  Don't start pollEsNodesVersion unless someone subscribes (elastic#56923)
  Do not write UUID file during optimize process (elastic#58899)
  [Endpoint] Task/add nav bar (elastic#58604)
  [Metric Alerts] Add backend support for multiple expressions per alert  (elastic#58672)
  [Metrics Alerts] Fix alerting on a rate aggregation (elastic#58789)
  disable flaky suite (elastic#55953)
  Revert "[SIEM] apollo@3 (elastic#51926)" and "[SIEM][CASE] Init Confi… (elastic#58806)
  [resubmit] Prep agg types for new platform (elastic#58893)
  [Lens] Allow number formatting within Lens (elastic#56253)
  [Autocomplete] Use settings from config rather than UI settings (elastic#58784)
  Improve action and trigger types (elastic#58657)
  ...

# Conflicts:
#	x-pack/plugins/upgrade_assistant/server/routes/reindex_indices/reindex_indices.ts
rudolf added a commit that referenced this pull request Mar 2, 2020
…8947)

* Don't start pollEsNodesVersion unless someone subscribes

By not polling until subscribed to, we prevent verbose error logs when
the optimizer is run (which automatically skips migrations).

* Test pollEsNodeVersions behaviour

* Cleanup unused code

* PR Feedback

* Make test more stable

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
rudolf added a commit that referenced this pull request Mar 2, 2020
…8946)

* Don't start pollEsNodesVersion unless someone subscribes

By not polling until subscribed to, we prevent verbose error logs when
the optimizer is run (which automatically skips migrations).

* Test pollEsNodeVersions behaviour

* Cleanup unused code

* PR Feedback

* Make test more stable

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:New Platform 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.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants