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

Add support for infinite ack timeout #107044

Closed
DaveCTurner opened this issue Apr 3, 2024 · 2 comments
Closed

Add support for infinite ack timeout #107044

DaveCTurner opened this issue Apr 3, 2024 · 2 comments
Labels
>bug :Distributed Coordination/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination.

Comments

@DaveCTurner
Copy link
Contributor

DaveCTurner commented Apr 3, 2024 •
edited
Loading

The ?master_node_timeout query param supports -1 to mean1 that the request waits forever without timing out. The ack timeout specified by the ?timeout query param also supports -1, but today this means the same as 0, i.e. that the wait for acks times out immediately. We should align the behaviour of these parameters so that users can specify that they want to wait forever for a cluster state update request to complete by setting them both to -1.

Footnotes

  1. unless there's currently no master, and some other corner cases, see https://github.com/elastic/elasticsearch/pull/107050 for fix ↩
@DaveCTurner DaveCTurner added >bug :Distributed Coordination/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. labels Apr 3, 2024
@elasticsearchmachine elasticsearchmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Apr 3, 2024
@elasticsearchmachine
Copy link
Collaborator

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

DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this issue Apr 19, 2024
@DaveCTurner
Make MasterServiceTests#testAcking deterministic
db3c463 
No need to use a real concurrent `ThreadPool` here, we can cover
everything we need with a fake threadpool, making the test deterministic
(and faster).

Relates elastic#107044
@DaveCTurner DaveCTurner mentioned this issue Apr 19, 2024
Merged
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this issue Apr 19, 2024
@DaveCTurner
Rename timeout to ackTimeout
b4bfdc2 
Elasticsearch has various timeout parameters, each of which applies in a
different context. One commonly-encountered such timeout is
`AcknowledgedRequest#timeout()`. Despite its unqualified name, this
timeout has a very specific meaning. This commit renames all code usages
to `ackTimeout()` to better clarify what it's for.

Relates elastic#107044
@DaveCTurner DaveCTurner mentioned this issue Apr 19, 2024
Merged
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this issue Apr 19, 2024
@DaveCTurner
Forbid null ack timeout
2ba85dd 
Today a cluster state update task with `null` ack timeout implies the
timeout is zero. This implicit behaviour is trappy, so this commit
forbids it.

Relates elastic#107044
@DaveCTurner DaveCTurner mentioned this issue Apr 19, 2024
Merged
DaveCTurner added a commit that referenced this issue Apr 22, 2024
@DaveCTurner
Rename timeout to ackTimeout (#107652)
430d920 
Elasticsearch has various timeout parameters, each of which applies in a
different context. One commonly-encountered such timeout is
`AcknowledgedRequest#timeout()`. Despite its unqualified name, this
timeout has a very specific meaning. This commit renames all code usages
to `ackTimeout()` to better clarify what it's for.

Relates #107044
DaveCTurner added a commit that referenced this issue Apr 22, 2024
@DaveCTurner
Make MasterServiceTests#testAcking deterministic (#107651)
841a274 
No need to use a real concurrent `ThreadPool` here, we can cover
everything we need with a fake threadpool, making the test deterministic
(and faster).

Relates #107044
elasticsearchmachine pushed a commit that referenced this issue Apr 22, 2024
@DaveCTurner
Forbid null ack timeout (#107653)
c0417b3 
Today a cluster state update task with `null` ack timeout implies the
timeout is zero. This implicit behaviour is trappy, so this commit
forbids it.

Relates #107044
@DaveCTurner
Copy link
Contributor Author

Closed by #107675

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
>bug :Distributed Coordination/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@DaveCTurner @elasticsearchmachine