Skip to content

Refine size-your-shards wording#89081

Merged
elasticsearchmachine merged 5 commits intoelastic:mainfrom
DaveCTurner:2022-08-03-size-your-shards-wording
Aug 8, 2022
Merged

Refine size-your-shards wording#89081
elasticsearchmachine merged 5 commits intoelastic:mainfrom
DaveCTurner:2022-08-03-size-your-shards-wording

Conversation

@DaveCTurner
Copy link
Copy Markdown
Member

Clarify that the limits in the docs are absolute maxima that will avoid
things just breaking but won't necessarily give great performance.

Clarify that the limits in the docs are absolute maxima that will avoid
things just breaking but won't necessarily give great performance.
@DaveCTurner DaveCTurner added >docs General docs changes :Distributed/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. v8.2.4 v8.3.4 v8.4.1 v8.5.0 labels Aug 3, 2022
@elasticsearchmachine elasticsearchmachine added Team:Distributed Meta label for distributed team. Team:Docs Meta label for docs team labels Aug 3, 2022
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-docs (Team:Docs)

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

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

@DaveCTurner
Copy link
Copy Markdown
Member Author

Copy link
Copy Markdown
Contributor

@jakommo jakommo left a comment

Choose a reason for hiding this comment

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

Thanks a lot @DaveCTurner ❤️ This makes it much more clear now !

Copy link
Copy Markdown
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.

Looks good though I have a few more wishes.

[discrete]
[[shard-count-recommendation]]
==== Aim for 3000 indices or fewer per GB of heap memory on each master node
==== Aim for fewer than 3000 indices per GB of heap memory on each master node
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should reverse the sentence here to say

Aim for 1 GB of heap memory on each master per 3000 indices

It is a recommendation for handling the necessary amount of indices, not a recommendation for how many indices they should have.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I prefer this order for a couple of reasons:

  1. IMO it's more common that the user knows the size of their nodes and is trying to work out a suitable sharding and retention strategy to match. For instance on Cloud there's only a few different possible master heap sizes.

  2. We want to phrase this as a bound, not a target, and I find it less natural to say "aim for more than 1GB of heap per 3000 indices".

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

About 1, this seems to be the wrong way around and I'd like to encourage that in the heading.

About 2, we can perhaps call it:

Suggested change
==== Aim for fewer than 3000 indices per GB of heap memory on each master node
==== Masters should have at least 1 GB of heap per 3000 indices.

each dedicated master node should have at least 4GB of heap. For non-dedicated
master nodes, the same rule holds and should be added to the heap requirements
of the other roles of each node.
As a general rule of thumb, you should have fewer than 3000 indices per GB of
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we also reverse the sentence here to derive the GB from number of indices?

Copy link
Copy Markdown
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 though I'd still like the heading change. Added a suggestion, but do not want to hold back the other additional text.

[discrete]
[[shard-count-recommendation]]
==== Aim for 3000 indices or fewer per GB of heap memory on each master node
==== Aim for fewer than 3000 indices per GB of heap memory on each master node
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

About 1, this seems to be the wrong way around and I'd like to encourage that in the heading.

About 2, we can perhaps call it:

Suggested change
==== Aim for fewer than 3000 indices per GB of heap memory on each master node
==== Masters should have at least 1 GB of heap per 3000 indices.

@DaveCTurner DaveCTurner added auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) auto-backport-and-merge labels Aug 8, 2022
@elasticsearchmachine elasticsearchmachine merged commit c81f907 into elastic:main Aug 8, 2022
@DaveCTurner DaveCTurner deleted the 2022-08-03-size-your-shards-wording branch August 8, 2022 09:06
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Aug 8, 2022
Clarify that the limits in the docs are absolute maxima that will avoid
things just breaking but won't necessarily give great performance.
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Aug 8, 2022
Clarify that the limits in the docs are absolute maxima that will avoid
things just breaking but won't necessarily give great performance.
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Aug 8, 2022
Clarify that the limits in the docs are absolute maxima that will avoid
things just breaking but won't necessarily give great performance.
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

💚 Backport successful

Status Branch Result
8.2
8.3
8.4

elasticsearchmachine pushed a commit that referenced this pull request Aug 8, 2022
Clarify that the limits in the docs are absolute maxima that will avoid
things just breaking but won't necessarily give great performance.
elasticsearchmachine pushed a commit that referenced this pull request Aug 8, 2022
Clarify that the limits in the docs are absolute maxima that will avoid
things just breaking but won't necessarily give great performance.
elasticsearchmachine pushed a commit that referenced this pull request Aug 8, 2022
Clarify that the limits in the docs are absolute maxima that will avoid
things just breaking but won't necessarily give great performance.
@mark-vieira mark-vieira added v8.4.0 and removed v8.4.1 labels Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Distributed/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. >docs General docs changes Team:Distributed Meta label for distributed team. Team:Docs Meta label for docs team v8.2.4 v8.3.4 v8.4.0 v8.5.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants