Skip to content

Conversation

@DaveCTurner
Copy link
Contributor

  • Add types for all params
  • Remove mention of JDKs before 11
  • Clarify some wording

Co-authored-by: Stef Nestor [email protected]

@DaveCTurner DaveCTurner added >docs General docs changes :Distributed Coordination/Network Http and internode communication implementations v8.3.4 v8.4.1 v8.5.0 labels Jul 29, 2022
@elasticsearchmachine elasticsearchmachine added Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. Team:Docs Meta label for docs team labels Jul 29, 2022
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

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

- Add types for all params
- Remove mention of JDKs before 11
- Clarify some wording

Co-authored-by: Stef Nestor <[email protected]>
@DaveCTurner DaveCTurner force-pushed the 2022-07-29-network-setting-types branch from f73bd52 to 23427f4 Compare July 29, 2022 08:57
Copy link
Contributor

@lockewritesdocs lockewritesdocs left a comment

Choose a reason for hiding this comment

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

Left some suggested changes based on the setting type and the default values.

Comment on lines 130 to 133
Configures whether detailed errors may be returned in HTTP responses. Defaults
to `true`, which means that HTTP requests which include
<<common-options-error-options,`?error_trace` URI parameter>> will return a
detailed error message including a stack trace if they encounter an exception.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Configures whether detailed errors may be returned in HTTP responses. Defaults
to `true`, which means that HTTP requests which include
<<common-options-error-options,`?error_trace` URI parameter>> will return a
detailed error message including a stack trace if they encounter an exception.
Configures whether detailed errors are returned in HTTP responses. Defaults
to `true`, which means that HTTP requests that include the
<<common-options-error-options,`?error_trace`>> parameter will return a
detailed error message including a stack trace if they encounter an exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think may be is correct here. By default we don't return detailed errors even if http.detailed_errors.enabled: true - this setting just permits the use of the ?error_trace parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See 72be4ee for an alternative.

Defaults to `network.tcp.keep_alive`.
(<<static-cluster-setting,Static>>, boolean)
Configures the `SO_KEEPALIVE` option for this socket, which determines whether
it sends TCP keepalive probes. Defaults to `network.tcp.keep_alive`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
it sends TCP keepalive probes. Defaults to `network.tcp.keep_alive`.
it sends TCP keepalive probes. Defaults to `true`.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a boolean setting, which I believe defaults to true.

@DaveCTurner
Copy link
Contributor Author

Thanks @lockewritesdocs. The transport.* and http.* settings largely fall back to the network.* ones so the changes you've suggested are not quite accurate. For instance if you set network.tcp.keep_count: 5 then transport.tcp.keep_count defaults to 5, not to -1. That's why we say that they default to the fallback setting in each case.

Copy link
Contributor

@lockewritesdocs lockewritesdocs left a comment

Choose a reason for hiding this comment

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

Thanks for the explanation @DaveCTurner. I resolved the comments relating to suggested changes on those settings. Merge away 🦖

@DaveCTurner DaveCTurner merged commit d5ea39b into elastic:main Aug 1, 2022
@DaveCTurner DaveCTurner deleted the 2022-07-29-network-setting-types branch August 1, 2022 18:59
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Aug 1, 2022
Clean up network setting docs

- Add types for all params
- Remove mention of JDKs before 11
- Clarify some wording

Co-authored-by: Stef Nestor <[email protected]>
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.3 Commit could not be cherrypicked due to conflicts
8.4

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 88929

elasticsearchmachine pushed a commit that referenced this pull request Aug 1, 2022
Clean up network setting docs

- Add types for all params
- Remove mention of JDKs before 11
- Clarify some wording

Co-authored-by: Stef Nestor <[email protected]>

Co-authored-by: Stef Nestor <[email protected]>
@lockewritesdocs
Copy link
Contributor

@DaveCTurner, it looks like the 8.3 backport failed.

DaveCTurner added a commit that referenced this pull request Aug 1, 2022
Clean up network setting docs

- Add types for all params
- Remove mention of JDKs before 11
- Clarify some wording

Co-authored-by: Stef Nestor <[email protected]>
@DaveCTurner
Copy link
Contributor Author

Thanks, yes, I did a custom bespoke artisanal backport to 8.3 in a42fc90.

@lockewritesdocs
Copy link
Contributor

Thanks, yes, I did a custom bespoke artisanal backport to 8.3 in a42fc90.

I'm assuming that you physically lifted the changes into the 8.3 branch, and then used a vintage bellows to propagate them through the code base.

@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

: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 v8.3.4 v8.4.0 v8.5.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants