Skip to content

[GCE Discovery] Correcly handle large zones with 500 or more instances #83785

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
Merged
merged 4 commits into from
Feb 15, 2022
Merged

[GCE Discovery] Correcly handle large zones with 500 or more instances #83785

merged 4 commits into from
Feb 15, 2022

Conversation

@cdmbr
Copy link
Contributor

@cdmbr cdmbr commented Feb 10, 2022

discovery gce plugin has some issues while running in large pools of vms instances.

this pr attempts to solve it.

closes #83783

@elasticsearchmachine elasticsearchmachine added v8.2.0 external-contributor Pull request authored by a developer outside the Elasticsearch team labels Feb 10, 2022
@cdmbr cdmbr force-pushed the fix/discovery-gce branch 4 times, most recently from 404b466 to 5e9c25b Compare February 10, 2022 14:05
@DaveCTurner DaveCTurner marked this pull request as draft February 10, 2022 14:24
@DaveCTurner DaveCTurner added the :Distributed Coordination/Discovery-Plugins Anything related to our integration plugins with EC2, GCP and Azure label Feb 10, 2022
@DaveCTurner
Copy link
Contributor

DaveCTurner commented Feb 10, 2022

Thanks for the PR @cdmbr. Looks like you're still actively working on it so I've marked it as a draft for now. Please ping when it's ready for review. The following command will enable CI on your branch:

@elasticsearchmachine ok to test

@cdmbr
Copy link
Contributor Author

cdmbr commented Feb 10, 2022

thanks for the return @DaveCTurner !

I was just doing some lint fixes (forgot to run spotless 😅), but it seems that it is all set. I'm marking it as ready for review.

Question: when it merges, will it be back-ported for all previous versions ?

@cdmbr cdmbr marked this pull request as ready for review February 10, 2022 15:03
@elasticmachine elasticmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Feb 10, 2022
@elasticmachine
Copy link
Collaborator

elasticmachine commented Feb 10, 2022

Pinging @elastic/es-distributed (Team:Distributed)

@fcofdez fcofdez self-requested a review February 10, 2022 17:09
@arteam arteam self-assigned this Feb 14, 2022
@arteam
Copy link
Contributor

arteam commented Feb 14, 2022

@elasticmachine run elasticsearch-ci/packaging-tests-windows-sample

@arteam arteam added >bug and removed >enhancement labels Feb 14, 2022
@arteam
Copy link
Contributor

arteam commented Feb 14, 2022

@cdmbr Thank you very much for your contribution! The PR looks conceptually good, I've left a couple of minor comments.

Question: when it merges, will it be back-ported for all previous versions ?

Yes, it's possible to backport it to the 8.1, 8.0 and 7.17 branches, but, alas, not further.

@cdmbr cdmbr requested a review from arteam February 14, 2022 13:45
@cdmbr cdmbr force-pushed the fix/discovery-gce branch from b171215 to 1091b89 Compare February 14, 2022 14:01
@fcofdez
Copy link
Contributor

fcofdez commented Feb 15, 2022

@cdmbr Thanks for the contribution, it looks good, I've left a comment around the test that I think it should be addressed.

@cdmbr cdmbr requested a review from fcofdez February 15, 2022 13:51
@cdmbr cdmbr force-pushed the fix/discovery-gce branch from 96e7f7e to 24ca40f Compare February 15, 2022 14:26
@arteam arteam added auto-backport Automatically create backport pull requests when merged v8.1.1 v8.0.1 labels Feb 15, 2022
arteam
arteam approved these changes Feb 15, 2022
View reviewed changes
Copy link
Contributor

@arteam arteam left a comment

Choose a reason for hiding this comment

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

LGTM!

@cdmbr Thank you very much for working on this. I will wait for @fcofdez's response and then we should be good to merge this PR.

fcofdez
fcofdez approved these changes Feb 15, 2022
View reviewed changes
Copy link
Contributor

@fcofdez fcofdez left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Thanks for the PR @cdmbr!. @arteam would you take care of merging this and updating the changelog description?

@arteam arteam changed the title fix: discovery gce instance pagination [GCE Discovery] Correcly handle large zones with 500 or more instances Feb 15, 2022
@arteam arteam merged commit 2bcc03d into elastic:master Feb 15, 2022
@arteam arteam mentioned this pull request Feb 15, 2022
Merged
arteam pushed a commit to arteam/elasticsearch that referenced this pull request Feb 15, 2022
@cdmbr @arteam
[GCE Discovery] Correcly handle large zones with 500 or more instances (
b9e126d 
elastic#83785)

Discovery gce plugin has some issues while running in large pools of vms instances.

This pr attempts to solve it.

Closes elastic#83783
@arteam arteam mentioned this pull request Feb 15, 2022
Merged
arteam pushed a commit to arteam/elasticsearch that referenced this pull request Feb 15, 2022
@cdmbr @arteam
[GCE Discovery] Correcly handle large zones with 500 or more instances (
136d797 
elastic#83785)

Discovery gce plugin has some issues while running in large pools of vms instances.

This pr attempts to solve it.

Closes elastic#83783
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Feb 15, 2022 •
edited by arteam
Loading

💚 Backport successful

Status Branch Result
✅ 8.0
✅ 8.1
✅ 7.17

@elastic elastic deleted a comment from elasticsearchmachine Feb 15, 2022
@arteam arteam added the v7.17.1 label Feb 15, 2022
@elastic elastic deleted a comment from elasticsearchmachine Feb 15, 2022
arteam pushed a commit to arteam/elasticsearch that referenced this pull request Feb 15, 2022
@cdmbr @arteam
[7.17] [GCE Discovery] Correcly handle large zones with 500 or more i…
3ded434 
…nstances (elastic#83785)

Backports the following commits to 7.17:
 - [GCE Discovery] Correcly handle large zones with 500 or more instances (elastic#83785)
@arteam arteam mentioned this pull request Feb 15, 2022
Merged
@arteam
Copy link
Contributor

arteam commented Feb 15, 2022

@cdmbr Thank you very much for contributing to Elasticsearch!

arteam added a commit that referenced this pull request Feb 15, 2022
@arteam @cdmbr
[7.17] [GCE Discovery] Correcly handle large zones with 500 or more i…
dbb5c95 
…nstances (#83785) (#83981)

Backports the following commits to 7.17:
 - [GCE Discovery] Correcly handle large zones with 500 or more instances (#83785)

Co-authored-by: Claudio Marins <[email protected]>
arteam added a commit that referenced this pull request Feb 15, 2022
@arteam @cdmbr
[GCE Discovery] Correcly handle large zones with 500 or more instances (
47839fb 
#83785) (#83978)

Discovery gce plugin has some issues while running in large pools of vms instances.

This pr attempts to solve it.

Closes #83783

Co-authored-by: Claudio Marins <[email protected]>
arteam added a commit that referenced this pull request Feb 15, 2022
@arteam @cdmbr
[GCE Discovery] Correcly handle large zones with 500 or more instances (
e4cfe2c 
#83785) (#83979)

Discovery gce plugin has some issues while running in large pools of vms instances.

This pr attempts to solve it.

Closes #83783

Co-authored-by: Claudio Marins <[email protected]>
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Feb 16, 2022
@weizijun
Merge branch 'fix-none-tsdb-index-dimension-tests' of github.com:weiz…
7367932 
…ijun/elasticsearch into fix-none-tsdb-index-dimension-tests

* 'fix-none-tsdb-index-dimension-tests' of github.com:weizijun/elasticsearch: (37 commits)
  [docs] Mention JDK 17 in the Contributing docs (elastic#84018)
  Fix GeoIpDownloader startup during rolling upgrade (elastic#84000)
  Script: Fields API for Dense Vector (elastic#83550)
  Move InferenceConfigUpdate under VersionedNamedWriteable (elastic#84022)
  [ML] Fix license feature test cleanup (elastic#84020)
  Replace deprecated api in artifact transforms (elastic#84015)
  QL: Add leniency option to SQL CLI (elastic#83795)
  [Stack Monitoring] add kibana_stats version alias to -mb template (elastic#83930)
  Optimize spliterator for ImmutableOpenMap (elastic#83899)
  Feature usage actions for archive (elastic#83931)
  Use latch to speedup multi feature migration test (elastic#84007)
  Make action names available in NodeClient (elastic#83919)
  [DOCS] Re-add HTTP proxy setings from elastic#82737 (elastic#84001)
  Add CI matrix configuration for snapshot BWC versions (elastic#83990)
  Update YAML Rest tests to check for product header on all responses (elastic#83290)
  TSDB: Add time series aggs cancellation (elastic#83492)
  [DOCS] Fix percolate query headings (elastic#83988)
  [DOCS] Move tip for percolate query example (elastic#83972)
  Simplify LocalExporter cleaner function to fix failing tests (elastic#83812)
  [GCE Discovery] Correcly handle large zones with 500 or more instances (elastic#83785)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fcofdez fcofdez fcofdez approved these changes

+1 more reviewer

@arteam arteam arteam approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@arteam arteam

Labels

auto-backport Automatically create backport pull requests when merged >bug :Distributed Coordination/Discovery-Plugins Anything related to our integration plugins with EC2, GCP and Azure external-contributor Pull request authored by a developer outside the Elasticsearch team Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v7.17.1 v8.0.1 v8.1.1 v8.2.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Discovery GCE Pagination Lack

6 participants

@cdmbr @DaveCTurner @elasticmachine @arteam @fcofdez @elasticsearchmachine