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

[DOCS] Clarify Recovery Settings for Shard Relocation #40329

Merged
merged 4 commits into from
Apr 26, 2019
Merged

[DOCS] Clarify Recovery Settings for Shard Relocation #40329

merged 4 commits into from
Apr 26, 2019

Conversation

jrodewig
Copy link
Contributor

  • Adds a sentence clarifying that recovery settings also apply to shard relocation
  • Rewrites introduction to highlight when peer recovery occurs
  • Rewrites and reformats setting descriptions for scannability and clarity

Resolves #14539 🤞

@jrodewig jrodewig added >docs General docs changes :Distributed Indexing/Recovery Anything around constructing a new shard, either from a local or a remote source. v7.0.0 v8.0.0 v7.2.0 labels Mar 21, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@jrodewig jrodewig requested a review from kat257 March 25, 2019 18:53
@jrodewig
Copy link
Contributor Author

Hi @ppf2

Let me know if this addresses your concerns from #14539.

I also included some other rewrites for clarity, but I can break those out into another PR if wanted. Thanks!

@jrodewig jrodewig requested review from ywelsch and removed request for dnhatn April 2, 2019 19:48
ywelsch
ywelsch suggested changes Apr 4, 2019
View reviewed changes
Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

While we're already touching these docs here, maybe cross-link to [[cat-recovery]] which provides progress information about recoveries.

docs/reference/modules/indices/recovery.asciidoc Outdated
* Recreates a shard lost during node failure
* Relocates a shard to another node due to a cluster rebalance or changes to the
<<modules-cluster, shard allocation settings>>
* Restores a <<modules-snapshots, snapshot>>
Copy link
Contributor

@ywelsch ywelsch Apr 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

recovery from snapshot is different to peer recovery and has different settings. Let's leave it out here.

docs/reference/modules/indices/recovery.asciidoc Outdated

[float]
==== Peer recovery settings
You can use the following _expert_ settings to manage resources for peer
Copy link
Contributor

Choose a reason for hiding this comment

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

The first setting (indices.recovery.max_bytes_per_sec) is not an expert-level setting and is quite often adjusted. Let's separate it from the second one which should be attributed as expert-level and will very rarely need adjustment.

@jrodewig
[DOCS] Remove snapshot recovery reference.
d09877e 
Call out link to [[cat-recovery]].
Separate expert settings.
@jrodewig
Copy link
Contributor Author

jrodewig commented Apr 4, 2019 •
edited
Loading

Thanks for the review @ywelsch! I addressed your feedback in commit d09877e.

Here's a preview of the changes.

Let me know if you have any other concerns.

@jrodewig jrodewig requested a review from ywelsch April 4, 2019 19:48
@polyfractal polyfractal removed the v7.0.0 label Apr 9, 2019
@jrodewig
Copy link
Contributor Author

@ywelsch Let me know if these changes look okay to you.

Happy to close or rewrite if needed.

ywelsch
ywelsch approved these changes Apr 26, 2019
View reviewed changes
Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

Great, thank you! Sorry for the late reply here, I was on vacation.

@jrodewig
Copy link
Contributor Author

No worries! Thank you!

@jrodewig jrodewig merged commit 0225af4 into elastic:master Apr 26, 2019
@jrodewig jrodewig deleted the recovery-rewrite branch April 26, 2019 14:23
jrodewig added a commit that referenced this pull request Apr 26, 2019
@jrodewig
[DOCS] Clarify Recovery Settings for Shard Relocation (#40329)
6a7459f 
* Clarify that peer recovery settings apply to shard relocation

* Fix awkward wording of 1st sentence

* [DOCS] Remove snapshot recovery reference.
Call out link to [[cat-recovery]].
Separate expert settings.
jrodewig added a commit that referenced this pull request Apr 26, 2019
@jrodewig
[DOCS] Clarify Recovery Settings for Shard Relocation (#40329)
5dc243c 
* Clarify that peer recovery settings apply to shard relocation

* Fix awkward wording of 1st sentence

* [DOCS] Remove snapshot recovery reference.
Call out link to [[cat-recovery]].
Separate expert settings.
jrodewig added a commit that referenced this pull request Apr 26, 2019
@jrodewig
[DOCS] Clarify Recovery Settings for Shard Relocation (#40329)
eed0d9b 
* Clarify that peer recovery settings apply to shard relocation

* Fix awkward wording of 1st sentence

* [DOCS] Remove snapshot recovery reference.
Call out link to [[cat-recovery]].
Separate expert settings.
@jaymode jaymode added v7.0.1 and removed v7.0.2 labels Apr 29, 2019
akhil10x5 pushed a commit to akhil10x5/elasticsearch that referenced this pull request May 2, 2019
@jrodewig @akhil10x5
[DOCS] Clarify Recovery Settings for Shard Relocation (elastic#40329)
99f9a70 
* Clarify that peer recovery settings apply to shard relocation

* Fix awkward wording of 1st sentence

* [DOCS] Remove snapshot recovery reference.
Call out link to [[cat-recovery]].
Separate expert settings.
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
@jrodewig
[DOCS] Clarify Recovery Settings for Shard Relocation (elastic#40329)
96bf5c2 
* Clarify that peer recovery settings apply to shard relocation

* Fix awkward wording of 1st sentence

* [DOCS] Remove snapshot recovery reference.
Call out link to [[cat-recovery]].
Separate expert settings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ywelsch ywelsch ywelsch approved these changes

@debadair debadair Awaiting requested review from debadair

@bleskes bleskes Awaiting requested review from bleskes

@ppf2 ppf2 Awaiting requested review from ppf2

@kat257 kat257 Awaiting requested review from kat257

Assignees
No one assigned
Labels
:Distributed Indexing/Recovery Anything around constructing a new shard, either from a local or a remote source. >docs General docs changes v7.0.1 v7.2.0 v8.0.0-alpha1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Clarify recovery vs. relocation in documentation
6 participants
@jrodewig @elasticmachine @ywelsch @jakelandis @polyfractal @jaymode