Progressive delay per attempt.

[koresar]

This allows progressive delay in Producer.send(). For example:

producer.send(data, {
  retries: {
    attempts: 10,
    delay: 60000,
    delays: [1000, 2000, 5000, 10000, 30000]
  }
});

The send() attempt will happen: immediately, in 1, 2, 5, 10, 30, 60, 60, 60, 60 sec.

This is helpful in case kafka becomes unavailable for some reason, but the produces continue to send new messages. In case the kafka was unavailable for couple of seconds only, we'll make sure to send it sooner than later.

Feel free to review. I'll fix anything ASAP.

Thank you @oleksiyk for the amazing full featured module! Great job :)

[oleksiyk]

While discussing #59 I decided to add increasing delay for producer attempts. Something like attempt * options.retries.delay. This will probably be enough and easier to use than a implicit list of delays. What do you think?

[koresar]

It depends on the algorithm. If it is simple attempt * options.retries.delay then it is not flexible enough for us. In fact, any algorithm would not satisfy the need for all the people. Thus, the delays array is the best solution for most, if not all, the needs.

[oleksiyk]

I don't think it really needs an algorithm, its just a failure retries not something over complicated. And even in case it needs an algorithm I would prefer to provide a callback function option that will return delay for next attempt. Why would you need to it so strictly configured? I can't imagine these delay to be somehow synchronised with app monitoring or something.

[tjdavey]

I think the backoff approach implemented as part of #59 will be sufficient for most people's requirements including ours. I see no reason to require an array of values.

[koresar]

@oleksiyk if I change this PR from array to a callback (task, attempt) would that be merged?

[oleksiyk]

Frankly, I think this is an over complication. Callback return value should be verified, it should probably be promisified and its errors should somehow be handled. And all this to just calculate the delay? I really hesitate.

Can you please explain why you need it to be so controllable and why attempt * delay won't work for you?

p.s.
It can't be (task, attempt) because there is an array of tasks to retry, its probably (tasks, attempt).

[koresar]

Callback return value should be verified, it should probably be promisified and its errors should somehow be handled.

No, and no. No need value verification. Bluebird does that for us. Promisification is an unnecessary overcomplication too.

About attempt * delay... I just thought of it one more time. Yeah it should work fine for us. :)

So, I'm changing this PR to attempt * delay then?

[oleksiyk]

No, and no.

What I mean is that this is not acceptable:

return Promise.delay(options.retries.getNextAttemptDelay(attempt)).then...

Yes, the program won't crash if callback throws because the callback is called inside the Promise, but you can't verify callback return value here and you can't handle its errors. In order to do so you'll need to either explicitly promisify it or wrap in a .then() and then attach error handlers. Thats exactly what I meant.

So, I'm changing this PR to attempt * delay then?

Yes, please!

[koresar]

Done. Please check @oleksiyk

[oleksiyk]

I can't merge this.

• its not squashed
• its a mix of different changes (unrelated typos and code)
• its adding absolutely unnecessary private method
• the test only checks that at least required number of attempts were performed, not the total number of attempts

[koresar]

• GitHub new feature allows you to squash with a single click when you merge a PR.
• You got us very surprised with this statement. Sounds like: "Can't accept because you fixed it". Could you please reconsider that?
• Removed the method. Unit test had to be removed too as it was testing the method.
• The existing tests are complete enough I believe. The delays are tested in the 'should return an error for unknown partition/topic and retry 5 times'.

Could you please merge and publish?

Ми тут були дуже здивовані першому та другому пунктах, Олексію.

[oleksiyk]

Guys, its totally my right not to struggle reading through unrelated changes and commits before I decide to merge the PR.

As I said in my very first comment, I was going to implement progressive delay myself so I'm not open to spend any more time on the two lines code change.