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 configuring DeleteInactiveTopic setting in namespace policy #7598

Merged
merged 5 commits into from
Jul 28, 2020

Conversation

315157973
Copy link
Contributor

Fixes #7485

Motivation

Support configuring DeleteInactiveTopic setting in namespace policy

Modifications

Only the two parameters brokerDeleteInactiveTopicsMode and brokerDeleteInactiveTopicsMaxInactiveDurationSeconds support namespace policy. The parameters are changed to Map structure, the key is the namespace, and the value is the parameter value.
Such as: namespace1=delete_when_no_subscriptions, namespace2=delete_when_no_subscriptions.

In addition, there is a key name called default. If it is set, other namespaces that do not specify parameters will use this parameter.
Such as: default=delete_when_no_subscriptions

Verifying this change

InactiveTopicDeleteTest.java

(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

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): (yes / no)
  • The public API: (yes / no)
  • The schema: (yes / no / don't know)
  • The default values of configurations: (yes / no)
  • The wire protocol: (yes / no)
  • The rest endpoints: (yes / no)
  • The admin cli options: (yes / no)
  • Anything that affects deployment: (yes / no / don't know)

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
  • If a feature is not applicable for documentation, explain why?
  • If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

Copy link
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

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

@315157973 We should support settings in pulsar-admin namespaces not in the broker.conf. If the policy is set at the namespace level, we should apply the namespace level settings. Otherwise, use the broker level settings.

Copy link
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

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

The changes look good and I think there are some situation needs to be clarified:

  1. If users do not specify the namespace level policy, use broker level config as default.
  2. Users can clear the namespace level policy, after that, we should use broker level configurations, So recommend adding clear-inactive-topic-policy here.
  3. If the namespace level policy specified, after the broker restart, we should apply the namespace level policy.

@codelipenghui
Copy link
Contributor

/pulsarbot run-failure-checks

@merlimat
Copy link
Contributor

I think this should be part of the same structure that is already used for topic auto-creation, since auto-creation and auto-deletion are tightly related

@codelipenghui
Copy link
Contributor

@merlimat Do you mean we should change InactiveTopicPolicy to TopicAutoDeletionPolicy? And change the command in the namespaces to set-topic-auto-creation, get-topic-auto-creation?

@wolfstudy wolfstudy merged commit 00e3089 into apache:master Jul 28, 2020
@wolfstudy
Copy link
Member

I think this should be part of the same structure that is already used for topic auto-creation, since auto-creation and auto-deletion are tightly related

Sorry @merlimat I merged into this pull request by mistake. If necessary, can we revert this change or open a new issue to fix the corresponding problem?

wolfstudy pushed a commit that referenced this pull request Jul 29, 2020
…7598)

### Motivation

Support configuring DeleteInactiveTopic setting in namespace policy

### Modifications

Only the two parameters `brokerDeleteInactiveTopicsMode` and `brokerDeleteInactiveTopicsMaxInactiveDurationSeconds` support namespace policy. The parameters are changed to Map structure, the key is the namespace, and the value is the parameter value.
Such as: namespace1=delete_when_no_subscriptions, namespace2=delete_when_no_subscriptions.

In addition, there is a key name called `default`. If it is set, other namespaces that do not specify parameters will use this parameter.
Such as: default=delete_when_no_subscriptions

(cherry picked from commit 00e3089)
jerrypeng pushed a commit to jerrypeng/incubator-pulsar that referenced this pull request Aug 14, 2020
…pache#7598)

### Motivation

Support configuring DeleteInactiveTopic setting in namespace policy

### Modifications

Only the two parameters `brokerDeleteInactiveTopicsMode` and `brokerDeleteInactiveTopicsMaxInactiveDurationSeconds` support namespace policy. The parameters are changed to Map structure, the key is the namespace, and the value is the parameter value.
Such as: namespace1=delete_when_no_subscriptions, namespace2=delete_when_no_subscriptions.

In addition, there is a key name called `default`. If it is set, other namespaces that do not specify parameters will use this parameter.
Such as: default=delete_when_no_subscriptions
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Aug 24, 2020
…pache#7598)

### Motivation

Support configuring DeleteInactiveTopic setting in namespace policy

### Modifications

Only the two parameters `brokerDeleteInactiveTopicsMode` and `brokerDeleteInactiveTopicsMaxInactiveDurationSeconds` support namespace policy. The parameters are changed to Map structure, the key is the namespace, and the value is the parameter value.
Such as: namespace1=delete_when_no_subscriptions, namespace2=delete_when_no_subscriptions.

In addition, there is a key name called `default`. If it is set, other namespaces that do not specify parameters will use this parameter.
Such as: default=delete_when_no_subscriptions
@315157973 315157973 deleted the ns-topic branch September 3, 2020 15:52
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Sep 5, 2020
…pache#7598)

### Motivation

Support configuring DeleteInactiveTopic setting in namespace policy

### Modifications

Only the two parameters `brokerDeleteInactiveTopicsMode` and `brokerDeleteInactiveTopicsMaxInactiveDurationSeconds` support namespace policy. The parameters are changed to Map structure, the key is the namespace, and the value is the parameter value.
Such as: namespace1=delete_when_no_subscriptions, namespace2=delete_when_no_subscriptions.

In addition, there is a key name called `default`. If it is set, other namespaces that do not specify parameters will use this parameter.
Such as: default=delete_when_no_subscriptions
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.

Support configuring DeleteInactiveTopic setting in namespace policy
6 participants