For one retry, length of list of fixed intervals passed to NewSimpleBackoff should be 2

[askterm]

Which version of Elastic are you using?

[ ] elastic.v5 (for Elasticsearch 5.x)

Please describe the expected behaviour

NewSimpleBackoff(500) should enable one retry after 500ms.

Please describe the actual behaviour

NewSimpleBackoff(0, 500) enables one retry after 500ms. This is because of below code in Next method implementation of SimpleBackoff: if retry >= len(b.ticks) { return 0, false } This means if retry = 1 and length of the list is also 1, there will not be any retries.

Any steps to reproduce the behavior?

Create SimpleBackoff by calling NewSimpleBackoff(500). Stop elasticsearch. Periodic healthcheck marks all nodes as dead. Start elasticsearch before the next periodic healthcheck. Fire an elasticsearch query (using PerformRequestWithOptions) before next periodic healthcheck. Since all connections are dead, it will invoke healthcheck. Call still fails because no retries happen, even though SimpleBackoff was created with one fixed interval value.

[olivere]

Can you make a test similar to this one that fails?

[askterm]

Sure, I will add a test. Thanks!

[askterm]

In PerformRequestWithOptions, n is declared but not initialised. If there are no connections available, n is incremented. This means that while retrying, for first retry 1 is passed not 0. In the test that you have mentioned, retries start with 0.

[olivere]

I agree, that seems to be wrong. It should be -1 by default, or incremented after retrying.

[olivere]

This will need to be backported to 6.x and 5.x.