Fix support for infinite ?master_timeout#107050
Conversation
Specifying `?master_timeout=-1` on an API which performs a cluster state update means that the cluster state update task will never time out while waiting in the pending tasks queue. However this parameter is also re-used in a few places where a timeout of `-1` means something else, typically to timeout immediately. This commit fixes those places so that `?master_timeout=-1` consistently means to wait forever.
|
Documentation preview: |
|
Pinging @elastic/es-distributed (Team:Distributed) |
|
Hi @DaveCTurner, I've created a changelog YAML for you. |
| TransportNodesSnapshotsStatus.TYPE, | ||
| new TransportNodesSnapshotsStatus.Request(nodesIds.toArray(Strings.EMPTY_ARRAY)).snapshots(snapshots) | ||
| .timeout(request.masterNodeTimeout()), | ||
| .timeout(request.masterNodeTimeout().millis() < 0 ? null : request.masterNodeTimeout()), |
There was a problem hiding this comment.
Shouldn't we compare with -1 explicitly here and everywhere down below?
Since user can specify any negative number in theory. Or doc should say that any negative number works as infinite timeout (which sounds confusing)
There was a problem hiding this comment.
We only accept -1 when parsing a time value:
elasticsearch/libs/core/src/main/java/org/elasticsearch/core/TimeValue.java
Lines 375 to 384 in 0699c93
However the convention elsewhere in the code seems to be to compare .millis() < 0. I think we could make this neater for sure, but that's a job for another day. The docs definitely don't need to mention any lenience in this area.
Specifying `?master_timeout=-1` on an API which performs a cluster state update means that the cluster state update task will never time out while waiting in the pending tasks queue. However this parameter is also re-used in a few places where a timeout of `-1` means something else, typically to timeout immediately. This commit fixes those places so that `?master_timeout=-1` consistently means to wait forever.
Specifying
?master_timeout=-1on an API which performs a cluster stateupdate means that the cluster state update task will never time out
while waiting in the pending tasks queue. However this parameter is also
re-used in a few places where a timeout of
-1means something else,typically to timeout immediately. This commit fixes those places so that
?master_timeout=-1consistently means to wait forever.