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

Support delete inactive topic when subscriptions caught up #6077

Merged
merged 5 commits into from
Jan 19, 2020

Conversation

codelipenghui
Copy link
Contributor

@codelipenghui codelipenghui commented Jan 17, 2020

Motivation

Currently, pulsar support delete inactive topic which has no active producers and no subscriptions. This pull request is support to delete inactive topics that all subscriptions of the topic are caught up and no active producers/consumer.

Modifications

Expose inactive topic delete mode in broker.conf, future more we can support namespace level configuration for the inactive topic delete mode.

Verifying this change

New unit tests added for each inactive topic delete mode.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (no)
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

  • Does this pull request introduce a new feature? (no)

@codelipenghui codelipenghui requested review from sijie, merlimat and jiazhai and removed request for sijie and merlimat January 17, 2020 09:29
@codelipenghui codelipenghui self-assigned this Jan 17, 2020
@codelipenghui
Copy link
Contributor Author

run integration tests

@codelipenghui
Copy link
Contributor Author

run java8 tests
run integration tests

@codelipenghui
Copy link
Contributor Author

run cpp tests

Copy link
Member

@sijie sijie left a comment

Choose a reason for hiding this comment

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

@codelipenghui overall looks good. however we need to take care of two things:

  1. the backward compatibility of the newly introduced setting.
  2. what is the "inactive" behavior for subscriptions all caught up? IMO A topic should be treated as an inactive topic in delete_when_subscriptions_caught_up mode when meet all the following conditions:

a) no messages published for a while (larger than the max inactive duration).
b) no producers connected to this topic.
c) no backlog.

I don't think we need to check if there are consumers connected.


Also I noticed that you indicated that this pull request fixes #4824. However I don't think this pull request fixes the problem in #4824. So that would be considered to remove from the description. Otherwise when we close this pull request, it will accidentally close #4824

@codelipenghui
Copy link
Contributor Author

@sijie I have addressed your comments, please help take a look.

@sijie sijie added the doc-required Your PR changes impact docs and you will update later. label Jan 18, 2020
@sijie
Copy link
Member

sijie commented Jan 18, 2020

@codelipenghui the change looks good to me.

I labeled this issue as doc-required. Can you create a follow-up issue of adding documentation for these newly introduced features?

@codelipenghui
Copy link
Contributor Author

@sijie I have create issue #6087 to track the document for this PR and update the description of this PR.

@tuteng
Copy link
Member

tuteng commented Mar 21, 2020

Add label release-2.5.1, due #6310 dependency

tuteng pushed a commit to AmateurEvents/pulsar that referenced this pull request Mar 21, 2020
### Motivation

Currently, pulsar support delete inactive topic which has no active producers and no subscriptions. This pull request is support to delete inactive topics that all subscriptions of the topic are caught up and no active producers/consumer. 

### Modifications

Expose inactive topic delete mode in broker.conf, future more we can support namespace level configuration for the inactive topic delete mode.

(cherry picked from commit dc7abd8)
tuteng pushed a commit that referenced this pull request Apr 13, 2020
### Motivation

Currently, pulsar support delete inactive topic which has no active producers and no subscriptions. This pull request is support to delete inactive topics that all subscriptions of the topic are caught up and no active producers/consumer. 

### Modifications

Expose inactive topic delete mode in broker.conf, future more we can support namespace level configuration for the inactive topic delete mode.

(cherry picked from commit dc7abd8)
jiazhai pushed a commit to jiazhai/pulsar that referenced this pull request May 18, 2020
### Motivation

Currently, pulsar support delete inactive topic which has no active producers and no subscriptions. This pull request is support to delete inactive topics that all subscriptions of the topic are caught up and no active producers/consumer.

### Modifications

Expose inactive topic delete mode in broker.conf, future more we can support namespace level configuration for the inactive topic delete mode.
(cherry picked from commit dc7abd8)
@Anonymitaet
Copy link
Member

@Anonymitaet Anonymitaet removed the doc-required Your PR changes impact docs and you will update later. label Jun 10, 2020
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Aug 24, 2020
### Motivation

Currently, pulsar support delete inactive topic which has no active producers and no subscriptions. This pull request is support to delete inactive topics that all subscriptions of the topic are caught up and no active producers/consumer. 

### Modifications

Expose inactive topic delete mode in broker.conf, future more we can support namespace level configuration for the inactive topic delete mode.
@codelipenghui codelipenghui deleted the inactive_topic branch May 19, 2021 05:31
@truong-hua
Copy link

@codelipenghui overall looks good. however we need to take care of two things:

  1. the backward compatibility of the newly introduced setting.
  2. what is the "inactive" behavior for subscriptions all caught up? IMO A topic should be treated as an inactive topic in delete_when_subscriptions_caught_up mode when meet all the following conditions:

a) no messages published for a while (larger than the max inactive duration). b) no producers connected to this topic. c) no backlog.

I don't think we need to check if there are consumers connected.

Also I noticed that you indicated that this pull request fixes #4824. However I don't think this pull request fixes the problem in #4824. So that would be considered to remove from the description. Otherwise when we close this pull request, it will accidentally close #4824

I agree that we not include the condition that consumers have to be inactive. The consumers usually be keep running to provide low latency message processing and so they are always active as much as possible, which will prevent the inactive topic to be cleaned up.

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

Successfully merging this pull request may close these issues.

None yet

5 participants