retryOnTimeout / retryBackoff:

[SSANSH]

On latest version of elastic-transport library, 2 new features are introduce

• "retryOnTimeout" = false
  d2956c2
• "retryBackoff" =
  (min: number, max: number, attempt: number): number {
  const ceiling = Math.min(max, 2 ** attempt) / 2
  return ceiling + ((Math.random() * (ceiling - min)) + min)
  }

https://github.com/elastic/elastic-transport-js/pull/101/files
This break how it should work on cluster.

🐛 Bug Report

if you have 3 elasticsearch node with maxretries = 4, if one query exit in timeout on node 1 , the connection pool will query the timeout query on node 2 and node 3 if need.
With this update, the query is run only on node 1.
Moreover this parameter cannot be updated on elastic client in order to keep how is running before.

Your Environment

• node version: v20.15.0
• @elastic/elasticsearch 8.14.0
• os: Linux

[SSANSH]

I found a way to disable by overload Transport class with a custom Class

class CustomTransport extends _elasticsearch.Transport {
 constructor(options) {
    // Modify the transport options or add custom settings
    const customOptions = {
      ...options,
      retryBackoff: () => 0,
      retryOnTimeout: true, 
    };
    
    super(customOptions);
  }
}

however I would like to understand if features added are really correct?

[SSANSH]

From my point of view we must have retrybackoff only when we are on same failure node and retry should always be apply

[JoshMock]

Creating a custom transport is indeed the appropriate way to override these features for now.

Are you saying that you would expect timeouts to be retried when there are nodes in the pool on which the request has not yet been tried?

[SSANSH]

yes exactly and in this case we dont need to apply backoff
you have 3 nodes, node 1 failed
we have to try node 2 and then node 3 without backoff

[JoshMock]

I'm not sure we should ever retry on timeout unless retryOnTimeout is true, just to make that option as explicit as possible. If you know your cluster has multiple nodes, and you've set maxRetries to >= the number of nodes, it seems reasonable to suggest that you explicitly set retryOnTimeout to true in that case.

But what I do see you saying is that retryBackoff creates an unnecessary delay when retries are enabled, when there are multiple nodes. To address this concern, it might make sense for us to update the logic to not start using retryBackoff until a retry has been attempted on every healthy node in the pool.

Does that solution make sense to you?

[SSANSH]

the change on retryOnTimeout default = false is a bc from my point of view, so this need to be define into the change log in minima.
for the retryBackoff , agree with the solution expose.

[JoshMock]

The retryBackoff issue discussed here is fixed in #135 and will be published to npm soon.