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 2022/10/27 08:22:36 UTC
[GitHub] [pulsar] tjiuming opened a new pull request, #18218: [feat][broker][admin][offload] Add offload managedLedgerOffloadThreshold RestAPI and CLI tools
tjiuming opened a new pull request, #18218:
URL: https://github.com/apache/pulsar/pull/18218
### Motivation
Add offload managedLedgerOffloadThreshold RestAPI and CLI tools
### Does this pull request potentially affect one of the following parts:
*If the box was checked, please highlight the changes*
- [ ] Dependencies (add or upgrade a dependency)
- [ ] The public API
- [ ] The schema
- [ ] The default values of configurations
- [ ] The threading model
- [ ] The binary protocol
- [x] The REST endpoints
- [x] The admin CLI options
- [ ] Anything that affects deployment
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc` <!-- Your PR contains doc changes. Please attach the local preview screenshots (run `sh start.sh` at `pulsar/site2/website`) to your PR description, or else your PR might not get merged. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
### Matching PR in forked repository
PR in forked repository: <!-- ENTER URL HERE -->
--
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] tjiuming commented on pull request #18218: [feat][admin] Add offload managedLedgerOffloadThreshold RestAPI and CLI tools
Posted by GitBox <gi...@apache.org>.
tjiuming commented on PR #18218:
URL: https://github.com/apache/pulsar/pull/18218#issuecomment-1319773273
/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] nodece commented on a diff in pull request #18218: [feat][admin] Add offload managedLedgerOffloadThreshold RestAPI and CLI tools
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #18218:
URL: https://github.com/apache/pulsar/pull/18218#discussion_r1010203387
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java:
##########
@@ -1398,15 +1398,18 @@ public void testSetOffloadThreshold() throws Exception {
System.out.println(namespace);
// set a default
pulsar.getConfiguration().setManagedLedgerOffloadAutoTriggerSizeThresholdBytes(1);
+ pulsar.getConfiguration().setManagedLedgerOffloadThresholdInSeconds(100);
// create the namespace
admin.namespaces().createNamespace(namespace, Set.of(testLocalCluster));
admin.topics().createNonPartitionedTopic(topicName.toString());
admin.namespaces().setOffloadDeleteLag(namespace, 10000, TimeUnit.SECONDS);
assertEquals(-1, admin.namespaces().getOffloadThreshold(namespace));
+ assertEquals(-1, admin.namespaces().getOffloadThresholdInSeconds(namespace));
// assert we get the default which indicates it will fall back to default
assertEquals(-1, admin.namespaces().getOffloadThreshold(namespace));
+ assertEquals(-1, admin.namespaces().getOffloadThresholdInSeconds(namespace));
Review Comment:
```suggestion
assertEquals(admin.namespaces().getOffloadThresholdInSeconds(namespace), -1);
```
--
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] nodece commented on a diff in pull request #18218: [feat][admin] Add offload managedLedgerOffloadThreshold RestAPI and CLI tools
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #18218:
URL: https://github.com/apache/pulsar/pull/18218#discussion_r1010210980
##########
pulsar-client-tools-test/src/test/java/org/apache/pulsar/admin/cli/PulsarAdminToolTest.java:
##########
@@ -804,11 +804,11 @@ public void namespaces() throws Exception {
verify(mockNamespaces).clearOffloadDeleteLag("myprop/clust/ns1");
namespaces.run(split(
- "set-offload-policies myprop/clust/ns1 -r test-region -d aws-s3 -b test-bucket -e http://test.endpoint -mbs 32M -rbs 5M -oat 10M -oae 10s -orp tiered-storage-first"));
+ "set-offload-policies myprop/clust/ns1 -r test-region -d aws-s3 -b test-bucket -e http://test.endpoint -mbs 32M -rbs 5M -oat 10M -aots 100 -oae 10s -orp tiered-storage-first"));
Review Comment:
Should be `-ts 100`?
--
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] Technoboy- merged pull request #18218: [feat][admin] Add offload managedLedgerOffloadThreshold RestAPI and CLI tools
Posted by GitBox <gi...@apache.org>.
Technoboy- merged PR #18218:
URL: https://github.com/apache/pulsar/pull/18218
--
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] nodece commented on a diff in pull request #18218: [feat][admin] Add offload managedLedgerOffloadThreshold RestAPI and CLI tools
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #18218:
URL: https://github.com/apache/pulsar/pull/18218#discussion_r1010195025
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -1965,6 +1965,36 @@ protected void internalSetOffloadThreshold(long newThreshold) {
}
}
+ protected CompletableFuture<Void> internalSetOffloadThresholdInSecondsAsync(long newThreshold) {
+ validateNamespacePolicyOperation(namespaceName, PolicyName.OFFLOAD, PolicyOperation.WRITE);
+ validatePoliciesReadOnlyAccess();
Review Comment:
Please switch to async.
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -1965,6 +1965,36 @@ protected void internalSetOffloadThreshold(long newThreshold) {
}
}
+ protected CompletableFuture<Void> internalSetOffloadThresholdInSecondsAsync(long newThreshold) {
+ validateNamespacePolicyOperation(namespaceName, PolicyName.OFFLOAD, PolicyOperation.WRITE);
Review Comment:
Please switch to async.
--
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] nodece commented on a diff in pull request #18218: [feat][admin] Add offload managedLedgerOffloadThreshold RestAPI and CLI tools
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #18218:
URL: https://github.com/apache/pulsar/pull/18218#discussion_r1010211283
##########
pulsar-client-tools-test/src/test/java/org/apache/pulsar/admin/cli/PulsarAdminToolTest.java:
##########
@@ -1414,11 +1421,11 @@ public void topicPolicies() throws Exception {
verify(mockGlobalTopicsPolicies).removeOffloadPolicies("persistent://myprop/clust/ns1/ds1");
cmdTopics.run(split("set-offload-policies persistent://myprop/clust/ns1/ds1 -d s3 -r" +
- " region -b bucket -e endpoint -m 8 -rb 9 -t 10 -orp tiered-storage-first -g"));
+ " region -b bucket -e endpoint -m 8 -rb 9 -t 10 -oats 100 -orp tiered-storage-first -g"));
Review Comment:
Should be `-ts 100`?
--
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] Technoboy- commented on a diff in pull request #18218: [feat][admin] Add offload managedLedgerOffloadThreshold RestAPI and CLI tools
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #18218:
URL: https://github.com/apache/pulsar/pull/18218#discussion_r1009035786
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -1965,6 +1965,32 @@ protected void internalSetOffloadThreshold(long newThreshold) {
}
}
+ protected void internalSetOffloadThresholdInSeconds(long newThreshold) {
Review Comment:
We should make this method to be async.
--
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] nodece commented on a diff in pull request #18218: [feat][admin] Add offload managedLedgerOffloadThreshold RestAPI and CLI tools
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #18218:
URL: https://github.com/apache/pulsar/pull/18218#discussion_r1010201029
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java:
##########
@@ -267,6 +275,97 @@ public void testOffloadPoliciesAppliedApi() throws Exception {
-> assertEquals(admin.topics().getOffloadPolicies(topicName, true), brokerPolicies));
}
+
+ @Test
+ public void testSetNamespaceOffloadPolicies() throws Exception {
+ conf.setManagedLedgerOffloadThresholdInSeconds(100);
+ conf.setManagedLedgerOffloadAutoTriggerSizeThresholdBytes(100);
+
+ OffloadPoliciesImpl policies = new OffloadPoliciesImpl();
+ policies.setManagedLedgerOffloadThresholdInBytes(200L);
+ policies.setManagedLedgerOffloadThresholdInSeconds(200L);
+ policies.setManagedLedgerOffloadDeletionLagInMillis(400L);
+ policies.setManagedLedgerOffloadDriver("s3");
+ policies.setManagedLedgerOffloadBucket("buck2");
+
+ admin.namespaces().setOffloadThresholdInSeconds(myNamespace, 300);
+ assertEquals(300, admin.namespaces().getOffloadThresholdInSeconds(myNamespace));
Review Comment:
```suggestion
assertEquals(admin.namespaces().getOffloadThresholdInSeconds(myNamespace), 300);
```
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java:
##########
@@ -267,6 +275,97 @@ public void testOffloadPoliciesAppliedApi() throws Exception {
-> assertEquals(admin.topics().getOffloadPolicies(topicName, true), brokerPolicies));
}
+
+ @Test
+ public void testSetNamespaceOffloadPolicies() throws Exception {
+ conf.setManagedLedgerOffloadThresholdInSeconds(100);
+ conf.setManagedLedgerOffloadAutoTriggerSizeThresholdBytes(100);
+
+ OffloadPoliciesImpl policies = new OffloadPoliciesImpl();
+ policies.setManagedLedgerOffloadThresholdInBytes(200L);
+ policies.setManagedLedgerOffloadThresholdInSeconds(200L);
+ policies.setManagedLedgerOffloadDeletionLagInMillis(400L);
+ policies.setManagedLedgerOffloadDriver("s3");
+ policies.setManagedLedgerOffloadBucket("buck2");
+
+ admin.namespaces().setOffloadThresholdInSeconds(myNamespace, 300);
+ assertEquals(300, admin.namespaces().getOffloadThresholdInSeconds(myNamespace));
+
+ admin.namespaces().setOffloadPolicies(myNamespace, policies);
+ assertEquals(policies, admin.namespaces().getOffloadPolicies(myNamespace));
Review Comment:
```suggestion
assertEquals(admin.namespaces().getOffloadPolicies(myNamespace), policies);
```
--
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] nodece commented on a diff in pull request #18218: [feat][admin] Add offload managedLedgerOffloadThreshold RestAPI and CLI tools
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #18218:
URL: https://github.com/apache/pulsar/pull/18218#discussion_r1010202607
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java:
##########
@@ -267,6 +275,97 @@ public void testOffloadPoliciesAppliedApi() throws Exception {
-> assertEquals(admin.topics().getOffloadPolicies(topicName, true), brokerPolicies));
}
+
+ @Test
+ public void testSetNamespaceOffloadPolicies() throws Exception {
+ conf.setManagedLedgerOffloadThresholdInSeconds(100);
+ conf.setManagedLedgerOffloadAutoTriggerSizeThresholdBytes(100);
+
+ OffloadPoliciesImpl policies = new OffloadPoliciesImpl();
+ policies.setManagedLedgerOffloadThresholdInBytes(200L);
+ policies.setManagedLedgerOffloadThresholdInSeconds(200L);
+ policies.setManagedLedgerOffloadDeletionLagInMillis(400L);
+ policies.setManagedLedgerOffloadDriver("s3");
+ policies.setManagedLedgerOffloadBucket("buck2");
+
+ admin.namespaces().setOffloadThresholdInSeconds(myNamespace, 300);
+ assertEquals(300, admin.namespaces().getOffloadThresholdInSeconds(myNamespace));
+
+ admin.namespaces().setOffloadPolicies(myNamespace, policies);
+ assertEquals(policies, admin.namespaces().getOffloadPolicies(myNamespace));
+
+ String topicName = testTopic + UUID.randomUUID();
+ try {
+ Topic topic = pulsar.getBrokerService().getOrCreateTopic(topicName).get(10, TimeUnit.SECONDS);
+ assertNotNull(topic);
+
+ assertTrue(topic instanceof PersistentTopic);
+
+ PersistentTopic persistentTopic = (PersistentTopic) topic;
+ ManagedLedger ledger = persistentTopic.getManagedLedger();
+ ManagedLedgerConfig config = ledger.getConfig();
+ OffloadPolicies policies1 = config.getLedgerOffloader().getOffloadPolicies();
+
+ assertEquals(policies1.getManagedLedgerOffloadThresholdInBytes(), policies.getManagedLedgerOffloadThresholdInBytes());
+ assertEquals(policies1.getManagedLedgerOffloadThresholdInSeconds(), policies.getManagedLedgerOffloadThresholdInSeconds());
+ } finally {
+ pulsar.getBrokerService().deleteTopic(topicName, true);
+ }
+ }
+
+ @Test
+ public void testSetTopicOffloadPolicies() throws Exception {
+ conf.setManagedLedgerOffloadThresholdInSeconds(100);
+ conf.setManagedLedgerOffloadAutoTriggerSizeThresholdBytes(100);
+
+ OffloadPoliciesImpl namespacePolicies = new OffloadPoliciesImpl();
+ namespacePolicies.setManagedLedgerOffloadThresholdInBytes(200L);
+ namespacePolicies.setManagedLedgerOffloadThresholdInSeconds(200L);
+ namespacePolicies.setManagedLedgerOffloadDeletionLagInMillis(400L);
+ namespacePolicies.setManagedLedgerOffloadDriver("s3");
+ namespacePolicies.setManagedLedgerOffloadBucket("buck2");
+
+ admin.namespaces().setOffloadThresholdInSeconds(myNamespace, 300);
+ assertEquals(300, admin.namespaces().getOffloadThresholdInSeconds(myNamespace));
Review Comment:
```suggestion
assertEquals(admin.namespaces().getOffloadThresholdInSeconds(myNamespace), 300);
```
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java:
##########
@@ -267,6 +275,97 @@ public void testOffloadPoliciesAppliedApi() throws Exception {
-> assertEquals(admin.topics().getOffloadPolicies(topicName, true), brokerPolicies));
}
+
+ @Test
+ public void testSetNamespaceOffloadPolicies() throws Exception {
+ conf.setManagedLedgerOffloadThresholdInSeconds(100);
+ conf.setManagedLedgerOffloadAutoTriggerSizeThresholdBytes(100);
+
+ OffloadPoliciesImpl policies = new OffloadPoliciesImpl();
+ policies.setManagedLedgerOffloadThresholdInBytes(200L);
+ policies.setManagedLedgerOffloadThresholdInSeconds(200L);
+ policies.setManagedLedgerOffloadDeletionLagInMillis(400L);
+ policies.setManagedLedgerOffloadDriver("s3");
+ policies.setManagedLedgerOffloadBucket("buck2");
+
+ admin.namespaces().setOffloadThresholdInSeconds(myNamespace, 300);
+ assertEquals(300, admin.namespaces().getOffloadThresholdInSeconds(myNamespace));
+
+ admin.namespaces().setOffloadPolicies(myNamespace, policies);
+ assertEquals(policies, admin.namespaces().getOffloadPolicies(myNamespace));
+
+ String topicName = testTopic + UUID.randomUUID();
+ try {
+ Topic topic = pulsar.getBrokerService().getOrCreateTopic(topicName).get(10, TimeUnit.SECONDS);
+ assertNotNull(topic);
+
+ assertTrue(topic instanceof PersistentTopic);
+
+ PersistentTopic persistentTopic = (PersistentTopic) topic;
+ ManagedLedger ledger = persistentTopic.getManagedLedger();
+ ManagedLedgerConfig config = ledger.getConfig();
+ OffloadPolicies policies1 = config.getLedgerOffloader().getOffloadPolicies();
+
+ assertEquals(policies1.getManagedLedgerOffloadThresholdInBytes(), policies.getManagedLedgerOffloadThresholdInBytes());
+ assertEquals(policies1.getManagedLedgerOffloadThresholdInSeconds(), policies.getManagedLedgerOffloadThresholdInSeconds());
+ } finally {
+ pulsar.getBrokerService().deleteTopic(topicName, true);
+ }
+ }
+
+ @Test
+ public void testSetTopicOffloadPolicies() throws Exception {
+ conf.setManagedLedgerOffloadThresholdInSeconds(100);
+ conf.setManagedLedgerOffloadAutoTriggerSizeThresholdBytes(100);
+
+ OffloadPoliciesImpl namespacePolicies = new OffloadPoliciesImpl();
+ namespacePolicies.setManagedLedgerOffloadThresholdInBytes(200L);
+ namespacePolicies.setManagedLedgerOffloadThresholdInSeconds(200L);
+ namespacePolicies.setManagedLedgerOffloadDeletionLagInMillis(400L);
+ namespacePolicies.setManagedLedgerOffloadDriver("s3");
+ namespacePolicies.setManagedLedgerOffloadBucket("buck2");
+
+ admin.namespaces().setOffloadThresholdInSeconds(myNamespace, 300);
+ assertEquals(300, admin.namespaces().getOffloadThresholdInSeconds(myNamespace));
+
+ admin.namespaces().setOffloadPolicies(myNamespace, namespacePolicies);
+ assertEquals(namespacePolicies, admin.namespaces().getOffloadPolicies(myNamespace));
Review Comment:
```suggestion
assertEquals(admin.namespaces().getOffloadPolicies(myNamespace), namespacePolicies);
```
--
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] nodece commented on a diff in pull request #18218: [feat][admin] Add offload managedLedgerOffloadThreshold RestAPI and CLI tools
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #18218:
URL: https://github.com/apache/pulsar/pull/18218#discussion_r1010203570
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java:
##########
@@ -1415,15 +1418,21 @@ public void testSetOffloadThreshold() throws Exception {
OffloadPoliciesImpl.DEFAULT_MAX_BLOCK_SIZE_IN_BYTES,
OffloadPoliciesImpl.DEFAULT_READ_BUFFER_SIZE_IN_BYTES,
admin.namespaces().getOffloadThreshold(namespace),
+ admin.namespaces().getOffloadThresholdInSeconds(namespace),
pulsar.getConfiguration().getManagedLedgerOffloadDeletionLagMs(),
OffloadPoliciesImpl.DEFAULT_OFFLOADED_READ_PRIORITY));
ledgerConf.setLedgerOffloader(offloader);
assertEquals(ledgerConf.getLedgerOffloader().getOffloadPolicies().getManagedLedgerOffloadThresholdInBytes(),
Long.valueOf(-1));
+ assertEquals(ledgerConf.getLedgerOffloader().getOffloadPolicies().getManagedLedgerOffloadThresholdInSeconds(),
+ Long.valueOf(-1));
+
// set an override for the namespace
admin.namespaces().setOffloadThreshold(namespace, 100);
+ admin.namespaces().setOffloadThresholdInSeconds(namespace, 100);
assertEquals(100, admin.namespaces().getOffloadThreshold(namespace));
+ assertEquals(100, admin.namespaces().getOffloadThresholdInSeconds(namespace));
Review Comment:
```suggestion
assertEquals(admin.namespaces().getOffloadThresholdInSeconds(namespace), 100);
```
--
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] nodece commented on a diff in pull request #18218: [feat][admin] Add offload managedLedgerOffloadThreshold RestAPI and CLI tools
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #18218:
URL: https://github.com/apache/pulsar/pull/18218#discussion_r1010197640
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java:
##########
@@ -2088,6 +2088,57 @@ public void setOffloadThreshold(@PathParam("tenant") String tenant,
internalSetOffloadThreshold(newThreshold);
}
+ @GET
+ @Path("/{tenant}/{namespace}/offloadThresholdInSeconds")
+ @ApiOperation(value = "Maximum number of bytes stored on the pulsar cluster for a topic,"
+ + " before the broker will start offloading to longterm storage",
+ notes = "A negative value disables automatic offloading")
+ @ApiResponses(value = { @ApiResponse(code = 403, message = "Don't have admin permission"),
+ @ApiResponse(code = 404, message = "Namespace doesn't exist") })
+ public void getOffloadThresholdInSeconds(
+ @Suspended final AsyncResponse asyncResponse,
+ @PathParam("tenant") String tenant,
+ @PathParam("namespace") String namespace) {
+ validateNamespaceName(tenant, namespace);
+ validateNamespacePolicyOperationAsync(namespaceName, PolicyName.OFFLOAD, PolicyOperation.READ)
+ .thenCompose(__ -> getNamespacePoliciesAsync(namespaceName))
+ .thenAccept(policies -> {
+ if (policies.offload_policies == null) {
Review Comment:
```suggestion
if (policies.offload_policies != null) {
```
--
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] nodece commented on a diff in pull request #18218: [feat][admin] Add offload managedLedgerOffloadThreshold RestAPI and CLI tools
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #18218:
URL: https://github.com/apache/pulsar/pull/18218#discussion_r1010211673
##########
pulsar-client-tools-test/src/test/java/org/apache/pulsar/admin/cli/PulsarAdminToolTest.java:
##########
@@ -1701,10 +1708,10 @@ public void topics() throws Exception {
cmdTopics.run(split("remove-delayed-delivery persistent://myprop/clust/ns1/ds1"));
verify(mockTopics).removeDelayedDeliveryPolicy("persistent://myprop/clust/ns1/ds1") ;
- cmdTopics.run(split("set-offload-policies persistent://myprop/clust/ns1/ds1 -d s3 -r region -b bucket -e endpoint -m 8 -rb 9 -t 10 -orp tiered-storage-first"));
+ cmdTopics.run(split("set-offload-policies persistent://myprop/clust/ns1/ds1 -d s3 -r region -b bucket -e endpoint -oats 50 -m 8 -rb 9 -t 10 -orp tiered-storage-first"));
Review Comment:
Should be `-ts 50`?
--
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] nodece commented on a diff in pull request #18218: [feat][admin] Add offload managedLedgerOffloadThreshold RestAPI and CLI tools
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #18218:
URL: https://github.com/apache/pulsar/pull/18218#discussion_r1010203209
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java:
##########
@@ -1398,15 +1398,18 @@ public void testSetOffloadThreshold() throws Exception {
System.out.println(namespace);
// set a default
pulsar.getConfiguration().setManagedLedgerOffloadAutoTriggerSizeThresholdBytes(1);
+ pulsar.getConfiguration().setManagedLedgerOffloadThresholdInSeconds(100);
// create the namespace
admin.namespaces().createNamespace(namespace, Set.of(testLocalCluster));
admin.topics().createNonPartitionedTopic(topicName.toString());
admin.namespaces().setOffloadDeleteLag(namespace, 10000, TimeUnit.SECONDS);
assertEquals(-1, admin.namespaces().getOffloadThreshold(namespace));
+ assertEquals(-1, admin.namespaces().getOffloadThresholdInSeconds(namespace));
Review Comment:
```suggestion
assertEquals(admin.namespaces().getOffloadThresholdInSeconds(namespace), -1);
```
--
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] codecov-commenter commented on pull request #18218: [feat][admin] Add offload managedLedgerOffloadThreshold RestAPI and CLI tools
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #18218:
URL: https://github.com/apache/pulsar/pull/18218#issuecomment-1316278566
# [Codecov](https://codecov.io/gh/apache/pulsar/pull/18218?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#18218](https://codecov.io/gh/apache/pulsar/pull/18218?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5520e2b) into [master](https://codecov.io/gh/apache/pulsar/commit/79750231051db849350e3d35dd3706320466acac?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7975023) will **decrease** coverage by `8.69%`.
> The diff coverage is `59.37%`.
[](https://codecov.io/gh/apache/pulsar/pull/18218?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18218 +/- ##
============================================
- Coverage 45.62% 36.92% -8.70%
+ Complexity 10075 1968 -8107
============================================
Files 697 208 -489
Lines 68024 14439 -53585
Branches 7293 1588 -5705
============================================
- Hits 31033 5331 -25702
+ Misses 33413 8526 -24887
+ Partials 3578 582 -2996
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests | `36.92% <59.37%> (-8.70%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pulsar/pull/18218?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...nt/impl/PulsarClientImplementationBindingImpl.java](https://codecov.io/gh/apache/pulsar/pull/18218/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL1B1bHNhckNsaWVudEltcGxlbWVudGF0aW9uQmluZGluZ0ltcGwuamF2YQ==) | `72.41% <ø> (-0.47%)` | :arrow_down: |
| [...ar/client/impl/conf/ConsumerConfigurationData.java](https://codecov.io/gh/apache/pulsar/pull/18218/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL2NvbmYvQ29uc3VtZXJDb25maWd1cmF0aW9uRGF0YS5qYXZh) | `92.55% <ø> (+2.12%)` | :arrow_up: |
| [...he/pulsar/client/impl/TypedMessageBuilderImpl.java](https://codecov.io/gh/apache/pulsar/pull/18218/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL1R5cGVkTWVzc2FnZUJ1aWxkZXJJbXBsLmphdmE=) | `30.76% <59.37%> (+3.56%)` | :arrow_up: |
| [...pache/pulsar/broker/admin/impl/NamespacesBase.java](https://codecov.io/gh/apache/pulsar/pull/18218/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi9pbXBsL05hbWVzcGFjZXNCYXNlLmphdmE=) | | |
| [.../org/apache/pulsar/broker/admin/v2/Namespaces.java](https://codecov.io/gh/apache/pulsar/pull/18218/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi92Mi9OYW1lc3BhY2VzLmphdmE=) | | |
| [...e/pulsar/broker/stats/AllocatorStatsGenerator.java](https://codecov.io/gh/apache/pulsar/pull/18218/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zdGF0cy9BbGxvY2F0b3JTdGF0c0dlbmVyYXRvci5qYXZh) | | |
| [...g/apache/bookkeeper/mledger/ManagedLedgerInfo.java](https://codecov.io/gh/apache/pulsar/pull/18218/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bWFuYWdlZC1sZWRnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWxlZGdlci9NYW5hZ2VkTGVkZ2VySW5mby5qYXZh) | | |
| [...e/pulsar/common/naming/NamespaceBundleFactory.java](https://codecov.io/gh/apache/pulsar/pull/18218/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NvbW1vbi9uYW1pbmcvTmFtZXNwYWNlQnVuZGxlRmFjdG9yeS5qYXZh) | | |
| [...ava/org/apache/bookkeeper/mledger/impl/OpScan.java](https://codecov.io/gh/apache/pulsar/pull/18218/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bWFuYWdlZC1sZWRnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWxlZGdlci9pbXBsL09wU2Nhbi5qYXZh) | | |
| [...he/bookkeeper/mledger/OffloadedLedgerMetadata.java](https://codecov.io/gh/apache/pulsar/pull/18218/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bWFuYWdlZC1sZWRnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWxlZGdlci9PZmZsb2FkZWRMZWRnZXJNZXRhZGF0YS5qYXZh) | | |
| ... and [486 more](https://codecov.io/gh/apache/pulsar/pull/18218/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
--
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