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

Align default behavior of @RetryableTopic and RetryTopicConfigurationBuilder with documentation #3706

Open
bky373 opened this issue Jan 12, 2025 · 4 comments
Milestone

Comments

@bky373
Copy link
Contributor

bky373 commented Jan 12, 2025

In what version(s) of Spring for Apache Kafka are you seeing this issue?

Since 3.2.0

Describe the bug

The default behavior of @RetryableTopic (SINGLE_TOPIC) differs from both RetryTopicConfigurationBuilder (MULTIPLE_TOPICS) and the Spring Kafka documentation, which describes using separate topics. Unifying all to SINGLE_TOPIC makes sense but needs documentation updates and clarification of changes.

  • Current result
$ kafka-topics --list --bootstrap-server localhost:9092
__consumer_offsets
retryable-annotation-topic
retryable-annotation-topic-dlt
retryable-annotation-topic-retry
  • Expected result (based on docs)
$ kafka-topics --list --bootstrap-server localhost:9092
__consumer_offsets
retryable-annotation-topic
retryable-annotation-topic-dlt
retryable-annotation-topic-retry-0
retryable-annotation-topic-retry-1

I would like to share three points regarding this issue.

  1. Inconsistency with Documentation
  2. Inconsistency with the Builder
  3. Direction for Defaults

1. Inconsistency with Documentation

The Spring Kafka documentation describes the default behavior for non-blocking retries as creating separate retry topics.

  • Example 1: How the Pattern Works

    If the message processing fails again the message will be forwarded to the next retry topic, and the pattern is repeated ..

  • Example 2: Topic Naming

    The default behavior is to create separate retry topics for each attempt, appended with an index value: retry-0, retry-1, …​, retry-n.

However, when using @RetryableTopic, the default behavior is to reuse the same topic for identical intervals. This means that without specifying any attributes (defaulting to FixedBackOff with attempts=3 and delay=1000ms), the retry topic will be reused. This change was introduced with PR 3058, which updated the default SameIntervalTopicReuseStrategy from MULTIPLE_TOPICS to SINGLE_TOPIC.
It would be ideal for the documentation and default behavior to be consistent.

2. Inconsistency with the Builder

As you know, non-blocking retries can also be configured using RetryTopicConfigurationBuilder instead of @RetryableTopic.
The default behavior of RetryTopicConfigurationBuilder.sameIntervalTopicReuseStrategy should align with the default behavior of @RetryableTopic to avoid confusion for users.

Currently, the builder’s default is still set to MULTIPLE_TOPICS, which is inconsistent with @RetryableTopic. The behavior of these two components should be unified.

3. Direction for Defaults

Referring to PR 2497, using SINGLE_TOPIC is more practical for exponential backoff scenarios. However, selecting SINGLE_TOPIC will also affect Fixed Backoff, as mentioned earlier.

If SINGLE_TOPIC is chosen as the default, the documentation regarding the default behavior should be revised.

Additionally, it’s crucial to highlight that this behavior differs significantly from previous versions.

While I’ve already updated the RetryTopicConfigurationBuilder.sameIntervalTopicReuseStrategy value to SINGLE_TOPIC and documented the changes, I want to ensure that this aligns with the intended direction. I’d appreciate your feedback on this matter.

To Reproduce

  • @RetryableTopic reuses the retry topic.
@Component
public class SimpleRetryableTopicListener {

    private static final Logger log = LoggerFactory.getLogger(SimpleRetryableTopicListener.class);

    @RetryableTopic
    @KafkaListener(
            id = "my-id",
            topics = "retryable-annotation-topic"
    )
    public void listen(String input) {
        log.info("--- Received input: {}", input);
        throw new RuntimeException("failed");
    }
}
  • RetryTopicConfigurationBuilder seperate the retry topics.
@Configuration
public class KafkaRetryConfig {

    @Bean
    public NewTopic newTopic() {
        return new NewTopic("retryable-annotation-topic", 1, (short) 1);
    }

    @Bean
    public RetryTopicConfiguration myRetryTopic() {
        Map<String, Object> props = new HashMap<>();
        props.put(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, "localhost:9092");

        return RetryTopicConfigurationBuilder
                .newInstance()
                .includeTopic("retryable-annotation-topic")
                .create(new KafkaTemplate<>(new DefaultKafkaProducerFactory<>(props)));
    }}
@artembilan
Copy link
Member

If we take a closer look into that PR, we will see that even @RetryableTopic was original with a SameIntervalTopicReuseStrategy.MULTIPLE_TOPICS.
Exactly same value as we still see in the RetryTopicConfigurationBuilder.

The change which made an inconsistency is this one: #3052.
Not clear why, but the cat is out of the bag.

And even if what you suggest makes sense, it is still a breaking change.
So, the issue will be revisited for upcoming 4.0 version.

Feel free to raise a PR and it will be reviewed and merged in respective timeline.

Thank you!

@bky373
Copy link
Contributor Author

bky373 commented Jan 14, 2025

@artembilan
Thanks for your comment.

I need to think about this a bit more. While reusing a single topic has its advantages, it also comes with disadvantages.

Let me share my thoughts on the pros and cons of using a single topic for retries.

Pros

1. Easier for beginners to understand and get started

  • Beginners don’t need to consider a topic suffix strategy when starting.
  • It’s suitable for use cases with a low number of retry attempts and simple logic.

2. Requires fewer resources for topic creation and management

  • Otherwise, retry topics need to be managed for each main topic up to (attempts - 1) (If retries are configured).
  • If there are many main topics, the additional retry topics could even outnumber the main topics in the list.

Cons

1. Potential delays in processing subsequent messages

  • Retries are performed sequentially within the same topic, which could cause delays for subsequent messages.
  • For example, the first retry attempt for each message might get continuously postponed. (Actually, not only the first retry attempt but also subsequent retries for a message might be continuously delayed.)

2. Difficulty in tracking retry attempts

  • Storing the same message repeatedly in the same topic makes it challenging to determine how many times a message has been retried.
  • This issue could be mitigated by using headers, but it adds complexity.

3. Other potential disadvantages

  • There may be other drawbacks I haven’t identified yet.

I am uncertain which approach would be better when considering these factors.
However, it is clear that a consistent policy must be maintained.

I would greatly appreciate your thoughts on this matter.
Depending on the outcome of the discussion, I will proceed with the necessary work.

@sobychacko
Copy link
Contributor

@bky373 we are thinking about starting the 4.0.x work in a few months. Once we have a branch ready, you can issue a PR and we can see what the pros/cons for the approach you are mentioning. Also, 4.0.x might be an opportunity for addressing any complexity in the retryable topic area for the users.

@bky373
Copy link
Contributor Author

bky373 commented Jan 15, 2025

@sobychacko
Yes, instead of making big changes to the current retry mechanism, I’m more focused on unifying the policy. This mainly involves a simple task of changing some default property values.

However, I agree it’s better to take time with this change since it could have some impact (even though parts of it are already applied). If I come up with a good idea before work on 4.0.x starts, I’ll be sure to share it with you.

Thank you for your reply!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants