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

[broker] topic closed normally will also run closeFencedTopicForcefully #15196

Closed
dragonls opened this issue Apr 18, 2022 · 0 comments · Fixed by #15202
Closed

[broker] topic closed normally will also run closeFencedTopicForcefully #15196

dragonls opened this issue Apr 18, 2022 · 0 comments · Fixed by #15202
Labels
type/bug The PR fixed a bug or issue reported a bug

Comments

@dragonls
Copy link
Contributor

Describe the bug
Pulsar cluster with topicFencingTimeoutSeconds>0, the topic which is closed normally should not having the log [A] Topic remained fenced for 10 seconds and is already closed (pendingWriteOps: 0).

To Reproduce
Steps to reproduce the behavior:

  1. Deply a cluster with topicFencingTimeoutSeconds > 0, for example topicFencingTimeoutSeconds=10
  2. Trigger topic A to close normally
  3. closeFencedTopicForcefully still will be run, as [A] Topic remained fenced for 10 seconds and is already closed (pendingWriteOps: 0) will be printed.

Expected behavior
If topic A close normally, [A] Topic remained fenced for 10 seconds and is already closed (pendingWriteOps: 0) should not be logged.

Desktop (please complete the following information):

  • OS: linux
  • Pulsar version: 2.8.3
@dragonls dragonls added the type/bug The PR fixed a bug or issue reported a bug label Apr 18, 2022
@dragonls dragonls changed the title [broker] topic closed normally may still try to closeFencedTopicForcefully [broker] topic closed normally will also run closeFencedTopicForcefully Apr 18, 2022
@dragonls dragonls reopened this Apr 18, 2022
michaeljmarshall pushed a commit that referenced this issue Apr 21, 2022
#15196) (#15202)

Co-authored-by: druidliu <[email protected]>

Fixes #15196.

### Motivation

If broker having conf `topicFencingTimeoutSeconds`>0, a topic is trigged closed and closed normally, `closeFencedTopicForcefully` should not be called.

### Modifications

Cancel fenced topic monitoring task if topic close normally, which cancel running `closeFencedTopicForcefully`.

### Verifying this change

- [ ] Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:
  - Add `org.apache.pulsar.broker.service.PersistentTopicTest#testTopicCloseFencingTimeout`


### 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

Check the box below or label this PR directly.

Need to update docs? 

- [ ] `doc-required` 
- [x] `no-need-doc` 
- [ ] `doc` 
- [ ] `doc-added`
michaeljmarshall pushed a commit that referenced this issue Apr 21, 2022
#15196) (#15202)

Co-authored-by: druidliu <[email protected]>

Fixes #15196.

### Motivation

If broker having conf `topicFencingTimeoutSeconds`>0, a topic is trigged closed and closed normally, `closeFencedTopicForcefully` should not be called.

### Modifications

Cancel fenced topic monitoring task if topic close normally, which cancel running `closeFencedTopicForcefully`.

### Verifying this change

- [ ] Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:
  - Add `org.apache.pulsar.broker.service.PersistentTopicTest#testTopicCloseFencingTimeout`

### 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

Check the box below or label this PR directly.

Need to update docs?

- [ ] `doc-required`
- [x] `no-need-doc`
- [ ] `doc`
- [ ] `doc-added`

(cherry picked from commit e4a8de1)
lhotari pushed a commit to datastax/pulsar that referenced this issue Apr 22, 2022
apache#15196) (apache#15202)

Co-authored-by: druidliu <[email protected]>

Fixes apache#15196.

### Motivation

If broker having conf `topicFencingTimeoutSeconds`>0, a topic is trigged closed and closed normally, `closeFencedTopicForcefully` should not be called.

### Modifications

Cancel fenced topic monitoring task if topic close normally, which cancel running `closeFencedTopicForcefully`.

### Verifying this change

- [ ] Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:
  - Add `org.apache.pulsar.broker.service.PersistentTopicTest#testTopicCloseFencingTimeout`

### 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

Check the box below or label this PR directly.

Need to update docs?

- [ ] `doc-required`
- [x] `no-need-doc`
- [ ] `doc`
- [ ] `doc-added`

(cherry picked from commit e4a8de1)
lhotari pushed a commit to datastax/pulsar that referenced this issue Apr 22, 2022
apache#15196) (apache#15202)

Co-authored-by: druidliu <[email protected]>

Fixes apache#15196.

If broker having conf `topicFencingTimeoutSeconds`>0, a topic is trigged closed and closed normally, `closeFencedTopicForcefully` should not be called.

Cancel fenced topic monitoring task if topic close normally, which cancel running `closeFencedTopicForcefully`.

- [ ] Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:
  - Add `org.apache.pulsar.broker.service.PersistentTopicTest#testTopicCloseFencingTimeout`

*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)

Check the box below or label this PR directly.

Need to update docs?

- [ ] `doc-required`
- [x] `no-need-doc`
- [ ] `doc`
- [ ] `doc-added`

(cherry picked from commit e4a8de1)
lhotari pushed a commit to datastax/pulsar that referenced this issue Apr 22, 2022
apache#15196) (apache#15202)

Co-authored-by: druidliu <[email protected]>

Fixes apache#15196.

If broker having conf `topicFencingTimeoutSeconds`>0, a topic is trigged closed and closed normally, `closeFencedTopicForcefully` should not be called.

Cancel fenced topic monitoring task if topic close normally, which cancel running `closeFencedTopicForcefully`.

- [ ] Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:
  - Add `org.apache.pulsar.broker.service.PersistentTopicTest#testTopicCloseFencingTimeout`

*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)

Check the box below or label this PR directly.

Need to update docs?

- [ ] `doc-required`
- [x] `no-need-doc`
- [ ] `doc`
- [ ] `doc-added`

(cherry picked from commit e4a8de1)
codelipenghui pushed a commit that referenced this issue Apr 28, 2022
#15196) (#15202)

Co-authored-by: druidliu <[email protected]>

Fixes #15196.

### Motivation

If broker having conf `topicFencingTimeoutSeconds`>0, a topic is trigged closed and closed normally, `closeFencedTopicForcefully` should not be called.

### Modifications

Cancel fenced topic monitoring task if topic close normally, which cancel running `closeFencedTopicForcefully`.

### Verifying this change

- [ ] Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:
  - Add `org.apache.pulsar.broker.service.PersistentTopicTest#testTopicCloseFencingTimeout`

### 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

Check the box below or label this PR directly.

Need to update docs?

- [ ] `doc-required`
- [x] `no-need-doc`
- [ ] `doc`
- [ ] `doc-added`

(cherry picked from commit e4a8de1)
codelipenghui pushed a commit that referenced this issue Apr 29, 2022
#15196) (#15202)

Co-authored-by: druidliu <[email protected]>

Fixes #15196.

### Motivation

If broker having conf `topicFencingTimeoutSeconds`>0, a topic is trigged closed and closed normally, `closeFencedTopicForcefully` should not be called.

### Modifications

Cancel fenced topic monitoring task if topic close normally, which cancel running `closeFencedTopicForcefully`.

### Verifying this change

- [ ] Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:
  - Add `org.apache.pulsar.broker.service.PersistentTopicTest#testTopicCloseFencingTimeout`

### 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

Check the box below or label this PR directly.

Need to update docs?

- [ ] `doc-required`
- [x] `no-need-doc`
- [ ] `doc`
- [ ] `doc-added`

(cherry picked from commit e4a8de1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
1 participant