You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/07/30 04:40:58 UTC
[GitHub] [pulsar] MarvinCai opened a new pull request #11509: Fix time based backlog quota.
MarvinCai opened a new pull request #11509:
URL: https://github.com/apache/pulsar/pull/11509
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:
- *Added unit test.*
### 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: yes
- 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
Added 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
- Doc should be auto generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] hangc0276 commented on a change in pull request #11509: Fix time based backlog quota.
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on a change in pull request #11509:
URL: https://github.com/apache/pulsar/pull/11509#discussion_r683456597
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/impl/BacklogQuotaImpl.java
##########
@@ -37,7 +38,7 @@ public static BacklogQuotaImplBuilder builder() {
}
public static class BacklogQuotaImplBuilder implements BacklogQuota.Builder {
- private long limitSize;
+ private long limitSize = -1 * 1024 * 1024 * 1024;
Review comment:
the default backlog limit size is -1GB, we use `-1 * GB` and define `GB = 1024 * 1024 * 1024` is better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] MarvinCai commented on pull request #11509: Fix time based backlog quota.
Posted by GitBox <gi...@apache.org>.
MarvinCai commented on pull request #11509:
URL: https://github.com/apache/pulsar/pull/11509#issuecomment-893969086
/pulsarbot run-failure-checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] hangc0276 merged pull request #11509: Fix time based backlog quota.
Posted by GitBox <gi...@apache.org>.
hangc0276 merged pull request #11509:
URL: https://github.com/apache/pulsar/pull/11509
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] MarvinCai commented on pull request #11509: Fix time based backlog quota.
Posted by GitBox <gi...@apache.org>.
MarvinCai commented on pull request #11509:
URL: https://github.com/apache/pulsar/pull/11509#issuecomment-893953690
/pulsarbot run-failure-checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] MarvinCai commented on a change in pull request #11509: Fix time based backlog quota.
Posted by GitBox <gi...@apache.org>.
MarvinCai commented on a change in pull request #11509:
URL: https://github.com/apache/pulsar/pull/11509#discussion_r683546186
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/impl/BacklogQuotaImpl.java
##########
@@ -37,7 +38,7 @@ public static BacklogQuotaImplBuilder builder() {
}
public static class BacklogQuotaImplBuilder implements BacklogQuota.Builder {
- private long limitSize;
+ private long limitSize = -1 * 1024 * 1024 * 1024;
Review comment:
actually as long as it's negative, it doesn't really matter, but just want to keep it consistent with existing default value: https://github.com/apache/pulsar/blob/27dcb0a6febd0d88438e18fae72aad8ee38c0738/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BacklogQuotaManager.java#L57
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] MarvinCai commented on pull request #11509: Fix time based backlog quota.
Posted by GitBox <gi...@apache.org>.
MarvinCai commented on pull request #11509:
URL: https://github.com/apache/pulsar/pull/11509#issuecomment-890485653
@Anonymitaet updated doc, please take a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on a change in pull request #11509: Fix time based backlog quota.
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #11509:
URL: https://github.com/apache/pulsar/pull/11509#discussion_r681359793
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/impl/BacklogQuotaImpl.java
##########
@@ -37,7 +38,7 @@ public static BacklogQuotaImplBuilder builder() {
}
public static class BacklogQuotaImplBuilder implements BacklogQuota.Builder {
- private long limitSize;
+ private long limitSize = -1 * 1024 * 1024 * 1024;
Review comment:
Why we need `-1 * 1024 * 1024 * 1024` not `-1`?
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/TopicPoliciesTest.java
##########
@@ -226,8 +282,56 @@ public void testCheckBacklogQuota() throws Exception {
admin.topics().deletePartitionedTopic(testTopic, true);
}
- @Test(timeOut = 20000)
- public void testGetBacklogQuotaApplied() throws Exception {
+ @Test
+ public void testCheckTimeBasedBacklogQuota() throws Exception {
+ RetentionPolicies retentionPolicies = new RetentionPolicies(10, 10);
+ String namespace = TopicName.get(testTopic).getNamespace();
+ admin.namespaces().setRetention(namespace, retentionPolicies);
+
+ Awaitility.await()
+ .untilAsserted(() -> Assert.assertEquals(admin.namespaces().getRetention(namespace), retentionPolicies));
+
+ BacklogQuota backlogQuota = BacklogQuota.builder()
+ .limitTime(10 * 60)
+ .retentionPolicy(BacklogQuota.RetentionPolicy.consumer_backlog_eviction)
+ .build();
+ log.info("Backlog quota: {} will set to the topic: {}", backlogQuota, testTopic);
+ try {
+ admin.topics().setBacklogQuota(testTopic, backlogQuota, BacklogQuota.BacklogQuotaType.message_age);
+ Assert.fail();
+ } catch (PulsarAdminException e) {
+ Assert.assertEquals(e.getStatusCode(), 412);
+ }
+
+ backlogQuota = BacklogQuota.builder()
+ .limitTime(10 * 60 + 1)
+ .retentionPolicy(BacklogQuota.RetentionPolicy.consumer_backlog_eviction)
+ .build();
+ log.info("Backlog quota: {} will set to the topic: {}", backlogQuota, testTopic);
+ try {
+ admin.topics().setBacklogQuota(testTopic, backlogQuota, BacklogQuota.BacklogQuotaType.message_age);
+ Assert.fail();
+ } catch (PulsarAdminException e) {
+ Assert.assertEquals(e.getStatusCode(), 412);
+ }
+
+ backlogQuota = BacklogQuota.builder()
+ .limitTime(10 * 60 - 1)
+ .retentionPolicy(BacklogQuota.RetentionPolicy.consumer_backlog_eviction)
+ .build();
+ log.info("Backlog quota: {} will set to the topic: {}", backlogQuota, testTopic);
+ admin.topics().setBacklogQuota(testTopic, backlogQuota, BacklogQuota.BacklogQuotaType.message_age);
+
+ BacklogQuota finalBacklogQuota = backlogQuota;
+ Awaitility.await()
+ .untilAsserted(() -> Assert.assertEquals(admin.topics().getBacklogQuotaMap(testTopic)
+ .get(BacklogQuota.BacklogQuotaType.message_age), finalBacklogQuota));
+
+ admin.topics().deletePartitionedTopic(testTopic, true);
+ }
+
+ @Test//(timeOut = 20000)
Review comment:
```suggestion
@Test(timeOut = 20000)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] MarvinCai commented on pull request #11509: Fix time based backlog quota.
Posted by GitBox <gi...@apache.org>.
MarvinCai commented on pull request #11509:
URL: https://github.com/apache/pulsar/pull/11509#issuecomment-893945109
/pulsarbot run-failure-checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org