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

[improve][client] Enhance dynamic topic subscription management in PatternMultiTopicsConsumer #23794

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Denovo1998
Copy link
Contributor

@Denovo1998 Denovo1998 commented Dec 30, 2024

Main Issue: #xyz

Motivation

When subscribing to topics using regex patterns, it’s possible to encounter scenarios where some topics process messages slower than others. This can lead to significant performance bottlenecks, as slower topics might block others. There’s a need to manage such situations dynamically to ensure efficient processing and fair resource allocation among topics.

Related discussions can be found in this Reddit thread, which highlights issues similar to what we aim to address.

When using MultiTopicsConsumerImpl, we can call this.

// un-subscribe a given topic
public CompletableFuture<Void> unsubscribeAsync(String topicName) {
checkArgument(TopicName.isValid(topicName), "Invalid topic name:" + topicName);
if (getState() == State.Closing || getState() == State.Closed) {
return FutureUtil.failedFuture(
new PulsarClientException.AlreadyClosedException("Topics Consumer was already closed"));
}
if (partitionsAutoUpdateTimeout != null) {
partitionsAutoUpdateTimeout.cancel();
partitionsAutoUpdateTimeout = null;
}

There is a test case that explains how to unsubscribe from a single topic.

// 4, unsubscribe topic3
CompletableFuture<Void> unsubFuture = ((MultiTopicsConsumerImpl<byte[]>) consumer).unsubscribeAsync(topicName3);
unsubFuture.get();

This proposal seeks to integrate similar functionality into PatternMultiTopicsConsumerImpl, allowing dynamically remove or add specified topics.

Modifications

  1. UnBlockTopics and BlockTopics Methods: In PatternMultiTopicsConsumerImpl, we introduce two methods: unBlockTopics and blockTopics.
  • BlockTopics: Unsubscribe from topics by invoking onTopicsRemoved from TopicsChangedListener.

  • UnBlockTopics: Resubscribe to topics by invoking onTopicsAdded from the same listener.

These changes will enable dynamic topic management, greatly improving the consumer’s ability to adapt to changing topic loads and enhancing overall system resilience against individual slow topics.

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

Documentation

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

Matching PR in forked repository

PR in forked repository:

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Dec 30, 2024
@Denovo1998 Denovo1998 changed the title Allow no longer to receive given topic messages in PatternMultiTopicsConsumerImpl [improve][client] Allow no longer to receive given topic messages in PatternMultiTopicsConsumerImpl Dec 30, 2024
@Denovo1998
Copy link
Contributor Author

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.

This PR appears to change the Pulsar client public API, even though the methods are protected. It remains unclear what problem this change is trying to solve. Instead of committing to a specific solution, it would be better to start a discussion on the [email protected] mailing list and reach consensus within the Pulsar project. After such a discussion, this would likely lead to creating a PIP (Pulsar Improvement Proposal).

@lhotari
Copy link
Member

lhotari commented Jan 3, 2025

@Denovo1998 I have shared details of how to use LLMs for improving PR titles and descriptions. That would help also for this PR since the title and description is confusing.
#23806 (comment)

@Denovo1998 Denovo1998 changed the title [improve][client] Allow no longer to receive given topic messages in PatternMultiTopicsConsumerImpl [improve][client] Enhance Topic Subscription Management in PatternMultiTopicsConsumerImpl for clients Jan 3, 2025
@Denovo1998 Denovo1998 changed the title [improve][client] Enhance Topic Subscription Management in PatternMultiTopicsConsumerImpl for clients [improve][client] Enhance dynamic topic subscription management in PatternMultiTopicsConsumer Jan 3, 2025
@Denovo1998
Copy link
Contributor Author

@lhotari Thank you, I've learned a little trick again. Technology changes life!

And I have just started a discussion on the mailing list.

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.

2 participants