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

Fix compaction not working for system topic #10941

Merged

Conversation

codelipenghui
Copy link
Contributor

Motivation

If a topic only have non-durable subscriptions but not durable subscriptions,
and the non-durable subscription reaches the end of the topic, we will get 0 estimated backlog size
So that the compaction will never be triggered.

The expected behavior is if we have no durable subscriptions, we should use the total size for triggering
the compaction.

Verifying this change

New tests added for ensuring the system topic been compacted.

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 introduces a new feature? (no)

If a topic only have non-durable subscriptions but not durable subscriptions,
and the non-durable subscription reach the end of the topic, we will get 0 estimated backlog size
So that the compaction will never been triggered.

The expected behavior is if we have no durable subscriptions, we should use the total size for triggering
the compaction.
Copy link
Contributor

@315157973 315157973 left a comment

Choose a reason for hiding this comment

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

Nice work. Do we need to add some non-durable subscription to verify it?

@codelipenghui
Copy link
Contributor Author

@315157973 No, the topic policy already created a reader for reading data from the system topic, so in the test I have
check the cacheIsInitialized first to make sure the reader been created.

Copy link
Contributor

@gaoran10 gaoran10 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 merged commit 797cb12 into apache:master Jun 17, 2021
@codelipenghui codelipenghui deleted the penghui/fix-compaction-system-topic branch June 17, 2021 07:08
yangl pushed a commit to yangl/pulsar that referenced this pull request Jun 23, 2021
If a topic only have non-durable subscriptions but not durable subscriptions,
and the non-durable subscription reach the end of the topic, we will get 0 estimated backlog size
So that the compaction will never been triggered.

The expected behavior is if we have no durable subscriptions, we should use the total size for triggering
the compaction.
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Jun 25, 2021
codelipenghui added a commit that referenced this pull request Jun 25, 2021
If a topic only have non-durable subscriptions but not durable subscriptions,
and the non-durable subscription reach the end of the topic, we will get 0 estimated backlog size
So that the compaction will never been triggered.

The expected behavior is if we have no durable subscriptions, we should use the total size for triggering
the compaction.

(cherry picked from commit 797cb12)
codelipenghui added a commit that referenced this pull request Jun 27, 2021
If a topic only have non-durable subscriptions but not durable subscriptions,
and the non-durable subscription reach the end of the topic, we will get 0 estimated backlog size
So that the compaction will never been triggered.

The expected behavior is if we have no durable subscriptions, we should use the total size for triggering
the compaction.

(cherry picked from commit 797cb12)
@codelipenghui codelipenghui added the cherry-picked/branch-2.7 Archived: 2.7 is end of life label Jun 27, 2021
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
If a topic only have non-durable subscriptions but not durable subscriptions,
and the non-durable subscription reach the end of the topic, we will get 0 estimated backlog size
So that the compaction will never been triggered.

The expected behavior is if we have no durable subscriptions, we should use the total size for triggering
the compaction.
@codelipenghui codelipenghui restored the penghui/fix-compaction-system-topic branch May 17, 2022 01:23
@codelipenghui codelipenghui deleted the penghui/fix-compaction-system-topic branch May 17, 2022 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-2.7 Archived: 2.7 is end of life cherry-picked/branch-2.8 Archived: 2.8 is end of life release/2.7.3 release/2.8.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants