-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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 time based backlog quota. #11509
Conversation
Time based backlog quota type message_age is set separately but when check backlog we're only checking against destination_storage type. So fix to loop through all BacklogQuotaType when checking if backlog exceeded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very good!
LGTM
probably we will have to update some docs (I am not sure)
@Anonymitaet updated doc, please take a look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just left some minor comments.
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/TopicPoliciesTest.java
Outdated
Show resolved
Hide resolved
...nt-admin-api/src/main/java/org/apache/pulsar/common/policies/data/impl/BacklogQuotaImpl.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job!
I left a comment.
...nt-admin-api/src/main/java/org/apache/pulsar/common/policies/data/impl/BacklogQuotaImpl.java
Outdated
Show resolved
Hide resolved
/pulsarbot run-failure-checks |
2 similar comments
/pulsarbot run-failure-checks |
/pulsarbot run-failure-checks |
Fixes apache#11404 ### Motivation Time based backlog quota type message_age is set separately but when check backlog we are only checking against destination_storage type. So fix to loop through all BacklogQuotaType when checking if backlog exceeded. ### Modification * Added unit test. * Added default implementation to make Admin Topic/Namespace backlog quota related API backward compatible.
Fixes #11404 ### Motivation Time based backlog quota type message_age is set separately but when check backlog we are only checking against destination_storage type. So fix to loop through all BacklogQuotaType when checking if backlog exceeded. ### Modification * Added unit test. * Added default implementation to make Admin Topic/Namespace backlog quota related API backward compatible. (cherry picked from commit e82df7c)
Fixes apache#11404 ### Motivation Time based backlog quota type message_age is set separately but when check backlog we are only checking against destination_storage type. So fix to loop through all BacklogQuotaType when checking if backlog exceeded. ### Modification * Added unit test. * Added default implementation to make Admin Topic/Namespace backlog quota related API backward compatible.
Fixes #11404
Motivation
Time based backlog quota type message_age is set separately but when check backlog we're only checking against destination_storage type.
So fix to loop through all BacklogQuotaType when checking if backlog exceeded.
Verifying this change
This change added tests and can be verified as follows:
Does this pull request potentially affect one of the following parts:
If
yes
was chosen, please highlight the changesAdded default implementation to make Admin Topic/Namespace backlog quota related API backward compatible.
Documentation
For contributor
For this PR, do we need to update docs? No