I suppose, I would like to see a version that can take sarama.NewExponentialBackoff(config) so that the appropriate fields from the config will be extracted and used without me needing to dig them out.

We probably also need a config.Producer.Retry.BackoffMax.

PS: I guess we cannot actually provide a simple config option, because each of the backoffs has its own sub-struct. 🤦‍♀️

That makes sense! I also considered making sarama.NewExponentialBackoff(config), but as you pointed out in the PS section, each backoff belongs to its own sub-struct, which makes a simple config-based approach impractical.

This is exactly why I made NewExponentialBackoff a standalone function—neither a method of Config nor a function that takes Config as an input parameter. This approach avoids ambiguity and ensures flexibility across different contexts (producer, metadata, transaction manager) without forcing unnecessary dependencies.

Regarding config.Producer.Retry.BackoffMax, I chose not to introduce it because it would require adding a similar field to every retry-related sub-struct (Metadata, TransactionManager), making the API more tedious to maintain. Keeping the backoff logic encapsulated in a separate function simplifies the design while still allowing explicit configuration per use case.

Let me know your thoughts!

I don’t know that there is a significantly stronger maintenance in adding the BackoffMax. Is it a lot of “toil”? Yes, but as you note, there is already a retry struct in a lot of the other config structs. So, the choice has kind of already been made.

It’s extremely important that when copy-pasting from a standard, you copy-paste it exactly. My point: the correct formula is:

MIN(retry.backoff.max.ms, (retry.backoff.ms * 2**(failures - 1)) * random(0.8, 1.2))

That word failures rather than retries is important here, because—bad news—the func(retries, maxRetries int) time.Duration function that we return? It’s being called with a decrementing retries, and not an incrementing failures. 😭

I’m not sure exactly how we can work around this generally… since the number of starting retries is potentially quite flexible (in particular: async_producer sets it to an initial pp.highwatermark -1).

Thanks for pointing this out! Here’s what I found:

Formula Correction

Updated the copied formula to exactly match KIP-580. Thanks for catching that!

Verification of retries Behavior in Sarama

Although pp.flushRetryBuffers flushes messages with decrementing retries, I ran tests and confirmed with additional logs that retries

• starts with 1 when pp.backoff(msg.retries) is called for the first time, and
• is incrementing,

at least in the producer case. Given this, the concern about decrementing retries shouldn’t apply in this context.

Flexible retries in Special Cases

The scenario where retries is more dynamic (e.g., pp.highwatermark - 1) appears to only occur for flagged (fin) messages being sent back to the input channel.

Given these findings, I believe our current implementation correctly follows KIP-580 in most cases. Let me know if you think any further checks are needed!

🤦‍♀️ I appear to have also missed where the function is receiving the maxRetries anyways. So, even if the retries were decrementing, we would still be able to determine the correct incrementing failures.

As noted from how how KIP-580 says defaults should be set, this should probably be:

if backoff <= 0 {
  backoff = defaultBackoff // 100 ms
}
if maxBackoff <= 0 {
  maxBackoff = defaultMaxBackoff // 1000 ms
}

As a single liner this looks like:

backoff = cmp.Or(max(backoff, 0), defaultBackoff)

Where the max(backoff, 0) clamps negatives to the zero value, which cmp.Or then skips over. Unfortunately, cmp.Or was introduced in go1.22.0, so not something we can use without bumping the go.mod go version.

Make sense. I choose the if statements.

I don’t know that there is a significantly stronger maintenance in adding the BackoffMax. Is it a lot of “toil”? Yes, but as you note, there is already a retry struct in a lot of the other config structs. So, the choice has kind of already been made.

👍