Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to Spring Kafka 3.0.0-M5 #31620

Closed
wilkinsona opened this issue Jul 7, 2022 · 6 comments
Closed

Upgrade to Spring Kafka 3.0.0-M5 #31620

wilkinsona opened this issue Jul 7, 2022 · 6 comments
Assignees
Labels
type: dependency-upgrade A dependency upgrade
Milestone

Comments

@wilkinsona
Copy link
Member

No description provided.

@wilkinsona wilkinsona added the type: dependency-upgrade A dependency upgrade label Jul 7, 2022
@wilkinsona wilkinsona added this to the 3.0.0-M4 milestone Jul 7, 2022
@wilkinsona
Copy link
Member Author

The latest changes break retries due to there being no bean named 'org.springframework.kafka.retrytopic.internalRetryTopicConfigurer'. We have this code at the moment:

@Bean
@ConditionalOnProperty(name = "spring.kafka.retry.topic.enabled")
@ConditionalOnSingleCandidate(KafkaTemplate.class)
public RetryTopicConfiguration kafkaRetryTopicConfiguration(KafkaTemplate<?, ?> kafkaTemplate) {
KafkaProperties.Retry.Topic retryTopic = this.properties.getRetry().getTopic();
RetryTopicConfigurationBuilder builder = RetryTopicConfigurationBuilder.newInstance()
.maxAttempts(retryTopic.getAttempts()).useSingleTopicForFixedDelays().suffixTopicsWithIndexValues()
.doNotAutoCreateRetryTopics();
setBackOffPolicy(builder, retryTopic);
return builder.create(kafkaTemplate);
}
private static void setBackOffPolicy(RetryTopicConfigurationBuilder builder, Topic retryTopic) {
long delay = (retryTopic.getDelay() != null) ? retryTopic.getDelay().toMillis() : 0;
if (delay > 0) {
PropertyMapper map = PropertyMapper.get().alwaysApplyingWhenNonNull();
BackOffPolicyBuilder backOffPolicy = BackOffPolicyBuilder.newBuilder();
map.from(delay).to(backOffPolicy::delay);
map.from(retryTopic.getMaxDelay()).as(Duration::toMillis).to(backOffPolicy::maxDelay);
map.from(retryTopic.getMultiplier()).to(backOffPolicy::multiplier);
map.from(retryTopic.isRandomBackOff()).to(backOffPolicy::random);
builder.customBackoff((SleepingBackOffPolicy<?>) backOffPolicy.build());
}
else {
builder.noBackoff();
}
}

I think we may need to auto-configure a RetryTopicConfigurationSupport sub-class that backs off when the user's defined their own directly or via @EnableKafkaRetryTopic. I'm not really sure what this would mean for our existing retry-related properties.

@garyrussell @tomazfernandes can you please help us here?

@tomazfernandes
Copy link
Contributor

Hi @wilkinsona, makes sense, sorry we missed that.

I think we may need to auto-configure a RetryTopicConfigurationSupport sub-class that backs off when the user's defined their own directly or via @EnableKafkaRetryTopic.

Perhaps @garyrussell and @artembilan might have a different idea, but this seems like the proper behavior to me. But forgive my ignorance - why would we need a sub-class instead of the class itself?

I'm not really sure what this would mean for our existing retry-related properties.

As long as we're able to autoconfigure the support bean, I don't think this should change anything - all the autoconfiguration does so far is add a RetryTopicConfiguration bean to the context. Of course, I might be missing something.

I'm not sure if that's something you or @garyrussell would like to fix yourselves - once we figure out the proper behavior I'm available to write a PR if needed.

Thanks.

@wilkinsona
Copy link
Member Author

wilkinsona commented Jul 8, 2022

all the autoconfiguration does so far is add a RetryTopicConfiguration bean to the context

I've yet to grok the difference between a RetryTopicConfigurationSupport sub-class and a RetryTopicConfiguration bean. There are enough similarities with Spring MVC to make the code feel familiar and enough differences for it to confuse me. At this stage, that's due to my lack of familiarity with the code more than anything.

@tomazfernandes
Copy link
Contributor

I've yet to grok the difference between a RetryTopicConfigurationSupport sub-class and a RetryTopicConfiguration bean. There are enough similarities with Spring MVC to make the code feel familiar and enough differences for it to confuse me. At this stage, that's due to my lack of familiarity with the code more than anything.

The RetryTopicConfigurationSupport subclassing is intended to let users configure framework-level features that will apply to the feature as a whole, such as global fatal exceptions and topic naming strategies, and have access to override the default infrastructure beans.

The RetryTopicConfiguration beans and @RetryableTopic annotations are specific configurations for given topics, and let users configure specific things like backoff, number of attempts, topic-specific exception classification, etc. The RetryTopicConfiguration can be applied to any given set of topics, while the annotation acts on the correspondent @KafkaListener topics.

Of course, perhaps the names could be clearer on its intentions, and probably the configuration strategy as a whole has room for improvement as well.

@garyrussell
Copy link
Contributor

This will affect Boot 2.7 users that wish to use s-k 2.9 as well.

Need to rethink.

@garyrussell
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: dependency-upgrade A dependency upgrade
Projects
None yet
Development

No branches or pull requests

3 participants