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

[ Issue 13479 ] Fixed internal topic effect by InactiveTopicPolicy. #13611

Merged
merged 6 commits into from
Jan 12, 2022
Merged

[ Issue 13479 ] Fixed internal topic effect by InactiveTopicPolicy. #13611

merged 6 commits into from
Jan 12, 2022

Conversation

mattisonchao
Copy link
Member

Fixes #13479

Master Issue: #13479

Motivation

see #13479

Modifications

  • make healthcheck internal topic as SystemTopic.

Verifying this change

  • Make sure that the change passes the CI checks.

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

none.

Documentation

  • no-need-doc

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jan 5, 2022
@mattisonchao
Copy link
Member Author

mattisonchao commented Jan 5, 2022

@codelipenghui @ericsyh @eolivelli @BewareMyPower @Technoboy-

PTAL :)

Another things:

Although this PR can fix the issue #13479, But I have some advice as below:

  1. healthcheck keyword name is common , maybe we can change this to some complex name.
  2. I think we can collect all internal or system theme names into one container, because they are currently everywhere. Unclear and difficult to maintain.

@mattisonchao
Copy link
Member Author

/pulsarbot rerun-failure-checks

@mattisonchao
Copy link
Member Author

/pulsarbot rerun-failure-checks

@mattisonchao
Copy link
Member Author

Copy link
Contributor

@Technoboy- Technoboy- left a comment

Choose a reason for hiding this comment

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

LGTM

@mattisonchao
Copy link
Member Author

/pulsarbot rerun-failure-checks

Copy link
Member

@RobertIndie RobertIndie left a comment

Choose a reason for hiding this comment

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

LGTM.

@codelipenghui codelipenghui added this to the 2.10.0 milestone Jan 12, 2022
@codelipenghui codelipenghui merged commit 5835191 into apache:master Jan 12, 2022
codelipenghui pushed a commit that referenced this pull request Jan 18, 2022
@codelipenghui codelipenghui added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Jan 18, 2022
@codelipenghui
Copy link
Contributor

@mattisonchao There are many conflicts with branch-2.8, could you please help push a new PR to fix the problem of branch-2.8?

@mattisonchao
Copy link
Member Author

mattisonchao commented Jan 18, 2022 via email

@mattisonchao
Copy link
Member Author

@codelipenghui

work done.
The new PR is #13816.

codelipenghui pushed a commit that referenced this pull request Mar 14, 2022
…gger compaction. (#14643)

### Motivation

When create persistent topic, create compaction subscription(line-1395) is before topic `initialize`(line-1397), it's better to do this after `initialize`:
https://github.com/apache/pulsar/blob/ad2cc2d38280b7dd0f056ee981ec8d3b157e3526/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L1391-L1398
If we change this part, we can optimize the heartbeat topic not trigger the compaction.
#13611 has made the heartbeat topic as a system topic to avoid deleting by GC. However, system topic is compacted by default. But the heartbeat topic sends msg without a key, so trigger compaction is meaningful.  So it's better to skip the heartbeat topic to do the compaction.

### Modification
- Move `preCreateSubscriptionForCompactionIfNeeded` after `initialize`.
- Add heartbeat topic not trigger compaction.
codelipenghui pushed a commit that referenced this pull request Apr 19, 2022
…gger compaction. (#14643)

### Motivation

When create persistent topic, create compaction subscription(line-1395) is before topic `initialize`(line-1397), it's better to do this after `initialize`:
https://github.com/apache/pulsar/blob/ad2cc2d38280b7dd0f056ee981ec8d3b157e3526/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L1391-L1398
If we change this part, we can optimize the heartbeat topic not trigger the compaction.
#13611 has made the heartbeat topic as a system topic to avoid deleting by GC. However, system topic is compacted by default. But the heartbeat topic sends msg without a key, so trigger compaction is meaningful.  So it's better to skip the heartbeat topic to do the compaction.

### Modification
- Move `preCreateSubscriptionForCompactionIfNeeded` after `initialize`.
- Add heartbeat topic not trigger compaction.

(cherry picked from commit d02bd73)
Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this pull request Apr 20, 2022
…gger compaction. (apache#14643)

### Motivation

When create persistent topic, create compaction subscription(line-1395) is before topic `initialize`(line-1397), it's better to do this after `initialize`:
https://github.com/apache/pulsar/blob/ad2cc2d38280b7dd0f056ee981ec8d3b157e3526/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L1391-L1398
If we change this part, we can optimize the heartbeat topic not trigger the compaction.
apache#13611 has made the heartbeat topic as a system topic to avoid deleting by GC. However, system topic is compacted by default. But the heartbeat topic sends msg without a key, so trigger compaction is meaningful.  So it's better to skip the heartbeat topic to do the compaction.

### Modification
- Move `preCreateSubscriptionForCompactionIfNeeded` after `initialize`.
- Add heartbeat topic not trigger compaction.
lhotari pushed a commit to datastax/pulsar that referenced this pull request May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.3 release/2.9.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug](broker): brokerDeleteInactiveTopic should exclude pulsar internal topics
6 participants