feat(retries): extend send msg api with retry cfg

[oetyng]

• This allows caller to apply a per-msg retry policy.
  BREAKING CHANGE: Removal of a public field plus Eq and PartialEq derivations on Config.

[grumbach]

Couple of (probably naive) questions

• Why not use the DEFAULT macros forDefault::default Trait?
• All the calls to send_message use None is the last arg really necessary?

[joshuef]

We need a BREAKING CHANGE: commit w/ the Eq/PartialEq being dropped

[connec]

Small API suggestion.

[connec]

Another reason I ended up with the min_retry_duration name is that the implementation uses it like a minimum:

match self.max_elapsed_time {
    Some(v) if self.get_elapsed_time() > v => None,
    _ => {
        let random = rand::random::<f64>();
        let randomized_interval = Self::get_random_value_from_interval(
            self.randomization_factor,
            random,
            self.current_interval,
        );
        self.current_interval = self.increment_current_interval();
        Some(randomized_interval)
    }
}

E.g., retry until more than max_elapsed_time has passed, so it's really the minimum amount of time.

[connec]

I think it could be argued that's a bug upstream.

[oetyng]

That's because it would be very bad to put

if self.get_elapsed_time() == v

..since you would then probably never hit that.

So, such cases always use >= or >, even though it's conceptually a max time that is modeled.

Max elapsed time is more accurate because there is not going to be any more time spent retrying than that.
If the retried call succeeds though, it can be a lot shorter than that, so Min is not very accurate.

I think they have it right there.

[connec]

I think the implementation is fine, it's just the name sets the wrong expectation for me. "Max elapsed time" makes me think it will not retry for longer than that, when in reality it will retry for at least that long, but in fact strictly longer.