Skip to content

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

Merged
codelipenghui merged 6 commits into
apache:masterfrom
mattisonchao:issue_13479
Jan 12, 2022
Merged

[ Issue 13479 ] Fixed internal topic effect by InactiveTopicPolicy.#13611
codelipenghui merged 6 commits into
apache:masterfrom
mattisonchao:issue_13479

Conversation

@mattisonchao

Copy link
Copy Markdown
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

mattisonchao commented Jan 5, 2022

Copy link
Copy Markdown
Member Author

@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
Copy Markdown
Member Author

/pulsarbot rerun-failure-checks

@mattisonchao

Copy link
Copy Markdown
Member Author

/pulsarbot rerun-failure-checks

@mattisonchao

Copy link
Copy Markdown
Member Author

@Technoboy- Technoboy- left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@mattisonchao

Copy link
Copy Markdown
Member Author

/pulsarbot rerun-failure-checks

@RobertIndie RobertIndie left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
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

mattisonchao commented Jan 18, 2022 via email

Copy link
Copy Markdown
Member Author

@mattisonchao

Copy link
Copy Markdown
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