Skip to content

Conversation

@zjxxzjwang
Copy link
Contributor

@zjxxzjwang zjxxzjwang commented Nov 24, 2025

Motivation

Currently, Pulsar Broker lacks a metric to track instances where delayed messages exceed their TTL. This results in messages set with delayed delivery times exceeding the TTL expiring before being consumed by users, with no mechanism to detect this occurrence. Consequently, there is a significant risk of message loss.

Modifications

  1. Add a new member variable Rate exceedTTLDelayMessage = new Rate() to PersistentTopic to track the number of delayed messages exceeding the TTL.
  2. Within the message sending logic of handleSend, add a check specifically for delayed messages whose delay time exceeds the TTL. If exceeded, call exceedTTLDelayMessage.recordEvent().
  3. In TopicStatsImpl, add the exceedTTLDelayMessages variable to retrieve and display the count of delayed messages exceeding the TTL.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@zjxxzjwang zjxxzjwang changed the title [improver][broker]Increase the number of delayed messages sent whose delay time exceeds the TTL statistical metric [improve][broker]Increase the number of delayed messages sent whose delay time exceeds the TTL statistical metric Nov 24, 2025
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Nov 24, 2025
Copy link
Contributor

@Denovo1998 Denovo1998 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments.

@zjxxzjwang
Copy link
Contributor Author

@Denovo1998 Hello, could you please help me review again

@zjxxzjwang
Copy link
Contributor Author

@Denovo1998 Hello, could you please help me review again

Comment on lines 4807 to 4809
if (deliverAtTime >= (messageTTLInSeconds * 1000L) + System.currentTimeMillis()) {
this.incrementExceedTTLDelayedMessages();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no guarantee that messageTTLInSeconds > 0.

I suggest wrapping the counting logic in

if (messageTTLInSeconds != null && messageTTLInSeconds > 0)

@Getter
private final PersistentTopicMetrics persistentTopicMetrics = new PersistentTopicMetrics();

private final Rate exceedTTLDelayedMessage = new Rate();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exceedTTLDelayedMessage doesn't read well. It seems that ttlExceededDelayedMessagesRate would be more accurate for this field.

In addition, it would be necessary to add a comment to this field to explain it.

Comment on lines +189 to +190
/** The number of delay messages that exceed TTL delay. */
public long exceedTTLDelayedMessages;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar comment about exceedTTLDelayedMessages as in the previous comment. It doesn't read well. I think that ttlExceededDelayedMessages would be better. In the comment, it should be made explicit that this is the number of delayed messsages that exceeded TTL at publish time.

Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the review comments.
In addition, please revisit the PR title. It's currently [improve][broker]Increase the number of delayed messages sent whose delay time exceeds the TTL statistical metric which is not very clear. Something like [improve][broker] Add counter to topic stats for delayed messages which exceed TTL at publish time would be more accurate.

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

Labels

doc-not-needed Your PR changes do not impact docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants