Closes #28450

This PR adds basic auto-configuration capabilities for Spring Kafka's non-blocking delayed retries feature (https://docs.spring.io/spring-kafka/reference/html/#retry-topic).

I chose not to add exception type classification to it just now as was previously suggested by @garyrussell so I can get feedback on the basic solution first. I can add exception type classification in a separate PR if and when this one is approved.

I wasn't sure whether to add an entry to the Messaging section of the documentation, since it seems to cover more basic use cases. I can do so if that's the case.

Seems like we should add the new properties to the A.8. Integration Properties section of the documentation, but I couldn't really find it in the source code. I can add it if you point me to it.

Please feel free to request any changes you see fit, or to make a polish commit on top of it.

As always, If @garyrussell and @artembilan can take a look into this it'd be great.

Thanks

Comment From: tomazfernandes

Thank you very much for the reviews. I've made the changes as suggested.

I removed the BackOff inner class since it'd have the same problem of creating a group with a dash in the name. Also, most of the BackOff creation logic and JavaDocs are from the Spring Retry project, not sure if we should add proper credit there somehow.

Let me know if there are any other suggestions or anything else to change.

Thanks.

Comment From: tomazfernandes

Implemented @artembilan's suggestions.

I looked into using @Valid to validate the input, but since a couple properties are Duration and there doesn't seem to be a way to properly validate since annotations don't allow such value types, I preferred to keep it simple and validate all properties together in the code.

Please let me know if there's a better way or anything else to change.

Thanks

Comment From: tomazfernandes

I've cleaned up the validation logic and added tests for that. Please let me know if there's anything else. Thanks!

Comment From: tomazfernandes

FYI, this unrelated test failed in the PR build:

Found test failures in 1 test task:
[00:00:08](https://ci.spring.io/builds/185388#L62321ef2:5476)

[00:00:08](https://ci.spring.io/builds/185388#L62321ef2:5477)
:spring-boot-project:spring-boot-devtools:test
[00:00:08](https://ci.spring.io/builds/185388#L62321ef2:5478)
    org.springframework.boot.devtools.tunnel.server.HttpTunnelServerTests > disconnectTimeout()`

I'll force-push a no-ops commit to trigger the PR build again. Thanks.

Comment From: tomazfernandes

@garyrussell, if and when possible, I'd like your input on the chosen sensible defaults, since it's something not so easy to change later.

Now we have:

useSingleTopicForFixedDelays() suffixTopicsWithIndexValues() doNotAutoCreateRetryTopics()

With these settings, by default users will get 3 topics, independently of the number of attempts or delay value: myTopic, myTopic-retry and myTopic-dlt

If a multiplier or maxDelay is set, yelding multiple delay values, users will get one topic per attempt plus the dlt: myTopic, myTopic-retry-0, myTopic-retry-1, myTopic-retry-2 ... myTopic-dlt

The other defaults are the same as for the rest of the feature, maxAttempts = 3 and FixedBackOff with delay=1s.

Thanks.

Comment From: tomazfernandes

Hi @snicoll and @wilkinsona, looks like from the Spring Kafka team's perspective there are no further concerns. Is there anything you'd like to be changed implementation-wise? We can revise any part of this based on user feedback for 2.7.

Thanks

Comment From: snicoll

Thanks for the follow-up and for working with the team.

Time is running out with RC1 happening tomorrow. I will do my best to review it before that happens if possible.

Comment From: tomazfernandes

Sure, no worries. I should be available if there's anything to be changed prior to the release. Thanks.

Comment From: snicoll

@tomazfernandes thank you for making your first contribution to Spring Boot. If you're interested, I've polished your contribution in b3e3581, mostly about using primitive types if possible. I also decided to remove the validation business as we don't really do such a thing in configuration properties usually. We'd rather let the underlying component we configure throw an exception if necessary.

Comment From: tomazfernandes

Thanks a lot @snicoll for finding the time to look into this in such short notice! The retryable topics feature has been gaining a lot of traction with the community and with auto configuration I'm sure more users will be able to benefit from it and in a simpler way.

I had missed that if users had more than one KafkaTemplate it'd throw an exception. If I understand correctly, now with @ConditionalOnSingleCandidate(KafkaTemplate.class) if users have more than one KafkaTemplate bean defined the retry topic configuration will back off. Is that correct?

I wonder if it would make sense to add a kafkaTemplate configuration property that defaults to "kafkaTemplate", and then fetch the template bean by name, so that users can still benefit of the feature by leveraging auto configuration's KafkaTemplate if that's the case, or set a different one to be used in case they have more than one defined. Is this something you usually do in auto configuration?

Or maybe that's expected behavior - if users choose to define multiple KafkaTemplate beans, they might just as well define RetryTopicConfiguration beans. We'd just need to properly document that.

What do you think?

Thanks again!

Comment From: snicoll

That's not what @ConditionalOnSingleCandidate does. A single KafkaTemplate should be defined (so either one in the context, or one flagged with @Primary in case of multiple instnaces). This is is consistent with what we do elsewhere and the recommendation is indeed to configure the feature if a "main" KafkaTemplate cannot be determined. The auto-configuration report will mention why the feature wasn't configured (again, something users rely on elsewhere) so I don't think it needs an explicit mention in the doc.

Comment From: tomazfernandes

I see, makes sense. I've learned a lot about autoconfiguration and what it is and isn't supposed to do in this issue, so thanks a lot everyone involved.

Thanks again @snicoll for looking into this in time for the RC release!