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

Remove unnecessary shuffle in unassigned shards allocation. #65172

Merged
merged 8 commits into from
Nov 23, 2021
Merged

Remove unnecessary shuffle in unassigned shards allocation. #65172

merged 8 commits into from
Nov 23, 2021

Conversation

howardhuanghua
Copy link
Contributor

We always sort unassigned shards before allocating them. Here are the sorting logics:

For exsiting shards allocation:

elasticsearch/server/src/main/java/org/elasticsearch/cluster/routing/allocation/AllocationService.java

Line 443 in caf9cd2

allocation.routingNodes().unassigned().sort(PriorityComparator.getAllocationComparator(allocation)); // sort for priority ordering

For new created shards or peer recovery shards allocation:

elasticsearch/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java

Line 797 in caf9cd2

ArrayUtil.timSort(primary, comparator);

So, the shuffle should be unnecessary before allocating them. This PR removes related unassigned shards shuffle operations.

@matriv matriv added the :Distributed Coordination/Allocation All issues relating to the decision making around placing a shard (both master logic & on the nodes) label Nov 25, 2020
@elasticmachine elasticmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Nov 25, 2020
@elasticmachine
Copy link
Collaborator

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

@howardhuanghua
Copy link
Contributor Author

howardhuanghua commented Nov 25, 2020 •
edited
Loading

Hi @matriv, thanks for checking PRs, would you please also help to find appropriate members to review this #64753 bug fix? I think it should be omitted.

@matriv
Copy link
Contributor

matriv commented Nov 26, 2020

@howardhuanghua

Both PRs are labeled/routed to the teams, so they'll get to them when they have the capacity to do so.

@howardhuanghua
Copy link
Contributor Author

@howardhuanghua

Both PRs are labeled/routed to the teams, so they'll get to them when they have the capacity to do so.

Thanks a lot @matriv .

@henningandersen
Copy link
Contributor

@elasticmachine check this please

@howardhuanghua
Copy link
Contributor Author

@henningandersen Would you please help to check this PR? I have updated the master branch. Thanks.

henningandersen
henningandersen approved these changes Nov 22, 2021
View reviewed changes
Copy link
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

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

LGTM.

@henningandersen
Copy link
Contributor

@elasticmachine check this please

@henningandersen henningandersen self-assigned this Nov 22, 2021
@henningandersen
Copy link
Contributor

@elasticmachine check this please

@henningandersen
Copy link
Contributor

@elasticmachine check this please

@howardhuanghua
Copy link
Contributor Author

Merged latest master branch. Thanks @henningandersen .

@henningandersen
Copy link
Contributor

@elasticmachine test this please

@henningandersen
Copy link
Contributor

@elasticmachine update branch

@howardhuanghua
Copy link
Contributor Author

Branch has been updated.

@henningandersen
Copy link
Contributor

@elasticmachine test this please

@elasticsearchmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@henningandersen
Copy link
Contributor

@elasticmachine run elasticsearch-ci/part-2

@henningandersen henningandersen merged commit 2ce9cd3 into elastic:master Nov 23, 2021
@henningandersen
Copy link
Contributor

Thanks for your contribution @howardhuanghua and sorry for the delay on progressing this.

@howardhuanghua
Copy link
Contributor Author

@henningandersen My pleasure, thank you.

weizijun added a commit to weizijun/elasticsearch that referenced this pull request Nov 23, 2021
@weizijun
Merge branch 'upstream/master' into move-tsdb-IT-to-yaml
0f15c86 
* upstream/master:
  Remove obsolete typed legacy index templates (elastic#80937)
  Remove unnecessary shuffle in unassigned shards allocation. (elastic#65172)
  TSDB: Tests for nanosecond timeprecision timestamp just beyond the limit (elastic#80932)

# Conflicts:
#	rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/10_settings.yml
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Nov 24, 2021
@weizijun
Merge branch 'upstream/master' into tsdb-required-range-settings
21b60b5 
* upstream/master: (29 commits)
  Fix typo (elastic#80925)
  Increase docker compose timeouts for CI builds
  TSDB: fix error without feature flag (elastic#80945)
  [DOCS] Relocate `index.mapping.dimension_fields.limit` setting docs (elastic#80964)
  Explicit write methods for always-missing values (elastic#80958)
  TSDB: move TimeSeriesModeIT to yaml tests (elastic#80933)
  [ML] Removing temporary debug (elastic#80956)
  Remove unused ConnectTransportException#node (elastic#80944)
  Reinterpret dots in field names as object structure (elastic#79922)
  Remove obsolete typed legacy index templates (elastic#80937)
  Remove unnecessary shuffle in unassigned shards allocation. (elastic#65172)
  TSDB: Tests for nanosecond timeprecision timestamp just beyond the limit (elastic#80932)
  Cleanup SLM History Item .equals (elastic#80938)
  Rework breaking changes for new structure (elastic#80907)
  [DOCS] Fix elasticsearch-reset-password typo (elastic#80919)
  [ML] No need to use parent task client when internal infer delegates (elastic#80905)
  Fix shadowed vars pt6 (elastic#80899)
  add ignore info (elastic#80924)
  Fix several potential circuit breaker leaks in Aggregators (elastic#79676)
  Extract more standard metadata from binary files (elastic#78754)
  ...
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Nov 24, 2021
@weizijun
Merge branch 'upstream/master' into update-filter-improve
8507e30 
* upstream/master: (319 commits)
  Fix typo (elastic#80925)
  Increase docker compose timeouts for CI builds
  TSDB: fix error without feature flag (elastic#80945)
  [DOCS] Relocate `index.mapping.dimension_fields.limit` setting docs (elastic#80964)
  Explicit write methods for always-missing values (elastic#80958)
  TSDB: move TimeSeriesModeIT to yaml tests (elastic#80933)
  [ML] Removing temporary debug (elastic#80956)
  Remove unused ConnectTransportException#node (elastic#80944)
  Reinterpret dots in field names as object structure (elastic#79922)
  Remove obsolete typed legacy index templates (elastic#80937)
  Remove unnecessary shuffle in unassigned shards allocation. (elastic#65172)
  TSDB: Tests for nanosecond timeprecision timestamp just beyond the limit (elastic#80932)
  Cleanup SLM History Item .equals (elastic#80938)
  Rework breaking changes for new structure (elastic#80907)
  [DOCS] Fix elasticsearch-reset-password typo (elastic#80919)
  [ML] No need to use parent task client when internal infer delegates (elastic#80905)
  Fix shadowed vars pt6 (elastic#80899)
  add ignore info (elastic#80924)
  Fix several potential circuit breaker leaks in Aggregators (elastic#79676)
  Extract more standard metadata from binary files (elastic#78754)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@henningandersen henningandersen henningandersen approved these changes

Assignees

@henningandersen henningandersen

Labels
:Distributed Coordination/Allocation All issues relating to the decision making around placing a shard (both master logic & on the nodes) >non-issue Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v8.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@howardhuanghua @elasticmachine @matriv @henningandersen @elasticsearchmachine