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.

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

Generalize TCP retxn docs to cover remote clusters #74732

Merged

Conversation

DaveCTurner
Copy link
Contributor

Today the docs on setting tcp_retries2 only talk about intra-cluster
connections, but in fact this setting is equally important to the
resilience of remote cluster connections too. This commit rewords these
docs to cover both cases.

Relates #34405

Today the docs on setting `tcp_retries2` only talk about intra-cluster
connections, but in fact this setting is equally important to the
resilience of remote cluster connections too. This commit rewords these
docs to cover both cases.

Relates elastic#34405
@DaveCTurner DaveCTurner added >docs General docs changes :Distributed Coordination/Network Http and internode communication implementations v8.0.0 v7.14.0 v7.13.3 labels Jun 30, 2021
@DaveCTurner DaveCTurner requested a review from jrodewig June 30, 2021 08:17
@elasticmachine elasticmachine added Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. Team:Docs Meta label for docs team labels Jun 30, 2021
@elasticmachine
Copy link
Collaborator

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

@elasticmachine
Copy link
Collaborator

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

Copy link
Contributor

@jrodewig jrodewig left a comment

Choose a reason for hiding this comment

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

LGTM. I left suggestions for a few non-blocking nits.

they can react promptly by reallocating lost shards, rerouting searches and
perhaps electing a new master node. Linux users should therefore reduce the
maximum number of TCP retransmissions.
very long periods of packet loss but this default is excessive and even harmful
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Compound sentences require a comma before the conjuction.

Suggested change
very long periods of packet loss but this default is excessive and even harmful
very long periods of packet loss, but this default is excessive and even harmful

perhaps electing a new master node. Linux users should therefore reduce the
maximum number of TCP retransmissions.
very long periods of packet loss but this default is excessive and even harmful
on the high quality networks used by most {es} installations. Highly-available
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: No hyphen needed.

Suggested change
on the high quality networks used by most {es} installations. Highly-available
on the high quality networks used by most {es} installations. Highly available

maximum number of TCP retransmissions.
very long periods of packet loss but this default is excessive and even harmful
on the high quality networks used by most {es} installations. Highly-available
clusters must be able to detect node failures promptly in order to react by
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: This sentence gets a bit long. I'd break it into two. Regardless, in order to can be shortened to to.

Suggested change
clusters must be able to detect node failures promptly in order to react by
clusters must be able to detect node failures promptly. This lets the cluster react by

very long periods of packet loss but this default is excessive and even harmful
on the high quality networks used by most {es} installations. Highly-available
clusters must be able to detect node failures promptly in order to react by
reallocating lost shards, rerouting searches and perhaps electing a new master
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: We tend to use an Oxford comma.

Suggested change
reallocating lost shards, rerouting searches and perhaps electing a new master
reallocating lost shards, rerouting searches, or electing a new master

reliability of communication with systems outside your cluster too. If your
cluster communicates with external systems over an unreliable network then you
reliability of communication with systems other than {es} clusters too. If your
clusters communicate with external systems over a low-quality network then you
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: No hyphen needed.

Suggested change
clusters communicate with external systems over a low-quality network then you
clusters communicate with external systems over a low quality network then you

or communication between the nodes is disrupted by a failure in the underlying
Each pair of {es} nodes communicates via a number of TCP connections which
<<long-lived-connections,remain open>> until one of the nodes shuts down or
communication between the nodes is disrupted by a failure in the underlying
infrastructure.

TCP provides reliable communication over occasionally-unreliable networks by
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: No hyphen is needed between an adverb and adjective.

Suggested change
TCP provides reliable communication over occasionally-unreliable networks by
TCP provides reliable communication over occasionally unreliable networks by

@DaveCTurner DaveCTurner merged commit 963d9aa into elastic:master Jul 5, 2021
@DaveCTurner DaveCTurner deleted the 2021-06-30-tcp-retries-remote-clusters branch July 5, 2021 12:38
DaveCTurner added a commit that referenced this pull request Jul 5, 2021
Today the docs on setting `tcp_retries2` only talk about intra-cluster
connections, but in fact this setting is equally important to the
resilience of remote cluster connections too. This commit rewords these
docs to cover both cases.

Relates #34405
DaveCTurner added a commit that referenced this pull request Jul 5, 2021
Today the docs on setting `tcp_retries2` only talk about intra-cluster
connections, but in fact this setting is equally important to the
resilience of remote cluster connections too. This commit rewords these
docs to cover both cases.

Relates #34405
DaveCTurner added a commit that referenced this pull request Jul 5, 2021
Today the docs on setting `tcp_retries2` only talk about intra-cluster
connections, but in fact this setting is equally important to the
resilience of remote cluster connections too. This commit rewords these
docs to cover both cases.

Relates #34405
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Coordination/Network Http and internode communication implementations >docs General docs changes Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. Team:Docs Meta label for docs team v7.13.4 v7.14.0 v7.15.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants