-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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
Provide explanation of dangling indices, fixes #26008 #26999
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few comments.
|
||
=== Dangling indices | ||
|
||
When a node joins the cluster, any shards/indices stored in its local `data/` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should refer to shards only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not see a reason to format "data/" here, we can say "data directory".
Adjusted from PR review comments
Thanks, I've adjusted with these suggestions. |
cluster. This functionality is intended as a best effort to help users who | ||
lost all master nodes. If a new master node is started which is unaware of | ||
the other indices in the cluster, adding the old nodes will cause the old | ||
indices to be imported, instead of being deleted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an extra "directory" in the first sentence.
I recommend avoiding future tense unless it's really necessary--and in this case I think it would be clearer to stick to present tense. I'd make the following tweaks:
When a node joins the cluster, any shards stored in its local data directory
that do not exist in the cluster are imported into the cluster. This functionality
is intended as a best effort to help you recover if a cluster loses all master
nodes. If a new master node is started that is unaware of the other indices in
the cluster, the old indices are imported as the old nodes join the cluster, not
deleted.
I reflexively use "that" instead of "which" for dependent clauses--though that "rule" isn't as hard and fast as some folks like to think.
We normally just keep pushing updates to doc PRs until it's ready to merge, then squash to a single commit and merge. Occasionally we'll keep multiple commits when merging, particularly if there are changes that are related, but not reflected in original commit messages.
LGTM as long as the directory stutter is fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@jasontedor I think I merged this one too quickly, can you review whenever you get some spare time, no rush on this either. Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the ping, I missed the previous one (sorry). I see that you merged but I left a comment about a duplicated word.
=== Dangling indices | ||
|
||
When a node joins the cluster, any shards stored in its local data directory | ||
directory which do not already exist in the cluster will be imported into the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The word directory
is duplicated from the previous line.
* master: Trim down usages of `ShardOperationFailedException` interface (#28312) Do not return all indices if a specific alias is requested via get aliases api. [Test] Lower bwc version for rank-eval rest tests CountedBitSet doesn't need to extend BitSet. (#28239) Calculate sum in Kahan summation algorithm in aggregations (#27807) (#27848) Remove the `update_all_types` option. (#28288) Add information when master node left to DiscoveryNodes' shortSummary() (#28197) Provide explanation of dangling indices, fixes #26008 (#26999)
Adding explanation of dangling indices back to the docs to fix #26008