You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "crossoverJie (via GitHub)" <gi...@apache.org> on 2023/09/21 15:24:53 UTC
[GitHub] [pulsar] crossoverJie opened a new pull request, #21228: [improve][pip] PIP-303: Add optional parameters for getPartitionedStats
crossoverJie opened a new pull request, #21228:
URL: https://github.com/apache/pulsar/pull/21228
Releted issue: https://github.com/apache/pulsar/issues/21200
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc` <!-- Your PR contains doc changes. -->
- [ ] `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 -->
--
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
Re: [PR] [improve][pip] PIP-303: Add optional parameters for getPartitionedStats [pulsar]
Posted by "crossoverJie (via GitHub)" <gi...@apache.org>.
crossoverJie commented on code in PR #21228:
URL: https://github.com/apache/pulsar/pull/21228#discussion_r1387442429
##########
pip/pip-303.md:
##########
@@ -0,0 +1,207 @@
+
+# Motivation
+
+When a topic has a large number of producers or consumers (over 1k), querying the `pulsarAdmin.topics().getPartitionedStats()` interface is slow and the response size is also large.
+As a result, it's essential to give users the option of querying producer and consumer information.
+
+
+
+# Goals
+
+## In Scope
+
+Add the API for `org.apache.pulsar.client.admin.Topics`
+```java
+CompletableFuture<PartitionedTopicStats> getPartitionedStatsAsync(
+ String topic, boolean perPartition, GetStatsOptions options);
+
+CompletableFuture<TopicStats> getStatsAsync(String topic, GetStatsOptions options);
+```
+
+
+
+## Out of Scope
+
+None.
+
+
+# High Level Design
+
+Implement the `getPartitionedStatsAsync` method, and add the `exclusivePublishers` and `exclusiveConsumers` parameters to `{tenant}/{namespace}/{topic}/partitioned-stats` API in `PersistentTopics` and `NonPersistentTopics`.
+
+# Detailed Design
+
+## Design & Implementation Details
+
+
+Add two fields for `org.apache.pulsar.client.admin.GetStatsOptions`
+```java
+@Data
+@Builder
+public class GetStatsOptions {
+ /**
+ * Whether to exclusive publishers.
+ */
+ private final boolean exclusivePublishers;
+
+ /**
+ * Whether to exclusive subscriptions.
+ */
+ private final boolean exclusiveConsumers;
+
+}
+```
+
+Implement the `getPartitionedStatsAsync` and `getStatsAsync` interface for `org.apache.pulsar.client.admin.internal.TopicsImpl`
+```java
+@Override
+public CompletableFuture<PartitionedTopicStats> getPartitionedStatsAsync(String topic,
+ boolean perPartition, boolean getPreciseBacklog, boolean subscriptionBacklogSize,boolean getEarliestTimeInBacklog,
Review Comment:
Done.
--
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
Re: [PR] [improve][pip] PIP-303: Add optional parameters for getPartitionedStats [pulsar]
Posted by "BewareMyPower (via GitHub)" <gi...@apache.org>.
BewareMyPower commented on code in PR #21228:
URL: https://github.com/apache/pulsar/pull/21228#discussion_r1386697447
##########
pip/pip-303.md:
##########
@@ -0,0 +1,207 @@
+
+# Motivation
+
+When a topic has a large number of producers or consumers (over 1k), querying the `pulsarAdmin.topics().getPartitionedStats()` interface is slow and the response size is also large.
+As a result, it's essential to give users the option of querying producer and consumer information.
+
+
+
+# Goals
+
+## In Scope
+
+Add the API for `org.apache.pulsar.client.admin.Topics`
+```java
+CompletableFuture<PartitionedTopicStats> getPartitionedStatsAsync(
+ String topic, boolean perPartition, GetStatsOptions options);
+
+CompletableFuture<TopicStats> getStatsAsync(String topic, GetStatsOptions options);
+```
+
+
+
+## Out of Scope
+
+None.
+
+
+# High Level Design
+
+Implement the `getPartitionedStatsAsync` method, and add the `exclusivePublishers` and `exclusiveConsumers` parameters to `{tenant}/{namespace}/{topic}/partitioned-stats` API in `PersistentTopics` and `NonPersistentTopics`.
+
+# Detailed Design
+
+## Design & Implementation Details
+
+
+Add two fields for `org.apache.pulsar.client.admin.GetStatsOptions`
+```java
+@Data
+@Builder
+public class GetStatsOptions {
+ /**
+ * Whether to exclusive publishers.
+ */
+ private final boolean exclusivePublishers;
+
+ /**
+ * Whether to exclusive subscriptions.
+ */
+ private final boolean exclusiveConsumers;
+
+}
+```
+
+Implement the `getPartitionedStatsAsync` and `getStatsAsync` interface for `org.apache.pulsar.client.admin.internal.TopicsImpl`
+```java
+@Override
+public CompletableFuture<PartitionedTopicStats> getPartitionedStatsAsync(String topic,
+ boolean perPartition, boolean getPreciseBacklog, boolean subscriptionBacklogSize,boolean getEarliestTimeInBacklog,
Review Comment:
The method signature is wrong.
--
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
Re: [PR] [improve][pip] PIP-303: Add optional parameters for getPartitionedStats [pulsar]
Posted by "crossoverJie (via GitHub)" <gi...@apache.org>.
crossoverJie commented on PR #21228:
URL: https://github.com/apache/pulsar/pull/21228#issuecomment-1801529137
@BewareMyPower I have updated the PIP, 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
Re: [PR] [improve][pip] PIP-303: Add optional parameters for getPartitionedStats [pulsar]
Posted by "Technoboy- (via GitHub)" <gi...@apache.org>.
Technoboy- merged PR #21228:
URL: https://github.com/apache/pulsar/pull/21228
--
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
Re: [PR] [improve][pip] PIP-303: Add optional parameters for getPartitionedStats [pulsar]
Posted by "nodece (via GitHub)" <gi...@apache.org>.
nodece commented on code in PR #21228:
URL: https://github.com/apache/pulsar/pull/21228#discussion_r1395722267
##########
pip/pip-303.md:
##########
@@ -0,0 +1,202 @@
+
+# Motivation
+
+When a topic has a large number of producers or consumers (over 1k), querying the `pulsarAdmin.topics().getPartitionedStats()` interface is slow and the response size is also large.
+As a result, it's essential to give users the option of querying producer and consumer information.
+
+
+
+# Goals
+
+## In Scope
+
+Add the API for `org.apache.pulsar.client.admin.Topics`
+```java
+CompletableFuture<PartitionedTopicStats> getPartitionedStatsAsync(
+ String topic, boolean perPartition, GetStatsOptions getStatsOptions);
+
+CompletableFuture<TopicStats> getStatsAsync(String topic, GetStatsOptions getStatsOptions);
+```
+
+
+
+## Out of Scope
+
+None.
+
+
+# High Level Design
+
+Implement the `getPartitionedStatsAsync` method, and add the `excludePublishers` and `excludeConsumers` parameters to `{tenant}/{namespace}/{topic}/partitioned-stats` API in `PersistentTopics` and `NonPersistentTopics`.
+
+# Detailed Design
+
+## Design & Implementation Details
+
+
+Add two fields for `org.apache.pulsar.client.admin.GetStatsOptions`
+```java
+@Data
+@Builder
+public class GetStatsOptions {
+ /**
+ * Whether to exclude publishers.
+ */
+ private final boolean excludePublishers;
+
+ /**
+ * Whether to exclude consumers.
+ */
+ private final boolean excludeConsumers;
+
+}
+```
+
+Implement the `getPartitionedStatsAsync` and `getStatsAsync` interface for `org.apache.pulsar.client.admin.internal.TopicsImpl`
+```java
+@Override
+public CompletableFuture<PartitionedTopicStats> getPartitionedStatsAsync(String topic, boolean perPartition, GetStatsOptions getStatsOptions){
+ TopicName tn = validateTopic(topic);
+ WebTarget path = topicPath(tn, "partitioned-stats");
+ path = path.queryParam("perPartition", perPartition)
+ .queryParam("getPreciseBacklog", getStatsOptions.isGetPreciseBacklog())
+ .queryParam("subscriptionBacklogSize", getStatsOptions.isSubscriptionBacklogSize())
+ .queryParam("getEarliestTimeInBacklog", getStatsOptions.isGetEarliestTimeInBacklog());
+ .queryParam("excludePublishers", getStatsOptions.isExcludePublishers())
+ .queryParam("excludeConsumers", getStatsOptions.isExcludeConsumers());
+}
+
+@Override
+public CompletableFuture<TopicStats> getStatsAsync(String topic, GetStatsOptions getStatsOptions){
+ TopicName tn = validateTopic(topic);
+ WebTarget path = topicPath(tn, "stats")
+ .queryParam("getPreciseBacklog", getStatsOptions.isGetPreciseBacklog())
+ .queryParam("subscriptionBacklogSize", getStatsOptions.isSubscriptionBacklogSize())
+ .queryParam("getEarliestTimeInBacklog", getStatsOptions.isGetEarliestTimeInBacklog());
+ .queryParam("excludePublishers",getStatsOptions.isExcludePublishers())
+ .queryParam("excludeConsumers",getStatsOptions.isExcludeConsumers());
+}
+```
+
+Add the `excludePublishers` and `excludeConsumers` parameters to `{tenant}/{namespace}/{topic}/partitioned-stats` API
+```java
+@GET
+@Path("{tenant}/{namespace}/{topic}/partitioned-stats")
+public void getPartitionedStats(
+ @Suspended final AsyncResponse asyncResponse,
+ @ApiParam(value = "Specify the tenant", required = true)
+ @PathParam("tenant") String tenant,
+ @ApiParam(value = "Specify the namespace", required = true)
+ @PathParam("namespace") String namespace,
+ @ApiParam(value = "Specify topic name", required = true)
+ @PathParam("topic") @Encoded String encodedTopic,
+ @ApiParam(value = "Get per partition stats")
+ @QueryParam("perPartition") @DefaultValue("true") boolean perPartition,
+ @ApiParam(value = "Whether leader broker redirected this call to this broker. For internal use.")
+ @QueryParam("authoritative") @DefaultValue("false") boolean authoritative,
+ @ApiParam(value = "If return precise backlog or imprecise backlog")
+ @QueryParam("getPreciseBacklog") @DefaultValue("false") boolean getPreciseBacklog,
+ @ApiParam(value = "If return backlog size for each subscription, require locking on ledger so be careful "
+ + "not to use when there's heavy traffic.")
+ @QueryParam("subscriptionBacklogSize") @DefaultValue("true") boolean subscriptionBacklogSize,
+ @ApiParam(value = "If return the earliest time in backlog")
+ @QueryParam("getEarliestTimeInBacklog") @DefaultValue("false") boolean getEarliestTimeInBacklog,
+ @ApiParam(value = "If exclude the publishers")
+ @QueryParam("excludePublishers") @DefaultValue("false") boolean excludePublishers,
+ @ApiParam(value = "If exclude the consumers")
+ @QueryParam("excludeConsumers") @DefaultValue("false") boolean excludeConsumers)
+
+```
+
+Add the `excludePublishers` and `excludeConsumers` parameters to `{tenant}/{namespace}/{topic}/stats` API
+```java
+@GET
+@Path("{tenant}/{namespace}/{topic}/stats")
+public void getStats(
+ @Suspended final AsyncResponse asyncResponse,
+ @ApiParam(value = "Specify the tenant", required = true)
+ @PathParam("tenant") String tenant,
+ @ApiParam(value = "Specify the namespace", required = true)
+ @PathParam("namespace") String namespace,
+ @ApiParam(value = "Specify topic name", required = true)
+ @PathParam("topic") @Encoded String encodedTopic,
+ @ApiParam(value = "Whether leader broker redirected this call to this broker. For internal use.")
+ @QueryParam("authoritative") @DefaultValue("false") boolean authoritative,
+ @ApiParam(value = "If return precise backlog or imprecise backlog")
+ @QueryParam("getPreciseBacklog") @DefaultValue("false") boolean getPreciseBacklog,
+ @ApiParam(value = "If return backlog size for each subscription, require locking on ledger so be careful "
+ + "not to use when there's heavy traffic.")
+ @QueryParam("subscriptionBacklogSize") @DefaultValue("true") boolean subscriptionBacklogSize,
+ @ApiParam(value = "If return time of the earliest message in backlog")
+ @QueryParam("getEarliestTimeInBacklog") @DefaultValue("false") boolean getEarliestTimeInBacklog,
+ @ApiParam(value = "If exclude the publishers"),
+ @QueryParam("excludePublishers") @DefaultValue("false") boolean excludePublishers,
+ @ApiParam(value = "If exclude the consumers")
+ @QueryParam("excludeConsumers") @DefaultValue("false") boolean excludeConsumers)
+```
+
+
+Add parameters for `org.apache.pulsar.broker.service.Topic`
+```java
+TopicStatsImpl getStats(boolean getPreciseBacklog, boolean subscriptionBacklogSize,
+ boolean getEarliestTimeInBacklog, boolean excludePublishers, boolean excludeConsumers);
+
+CompletableFuture<? extends TopicStatsImpl> asyncGetStats(boolean getPreciseBacklog,
Review Comment:
IMO, we only need to add an async method to avoid blocking the thread.
For the method arguments, we can also use the above design that moving these arguments to a class.
--
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
Re: [PR] [improve][pip] PIP-303: Add optional parameters for getPartitionedStats [pulsar]
Posted by "crossoverJie (via GitHub)" <gi...@apache.org>.
crossoverJie commented on PR #21228:
URL: https://github.com/apache/pulsar/pull/21228#issuecomment-1803207408
> Overall LGTM.
>
> I just think the name would better be "excludeXxxx" rather than "exclusiveXxx" because the logic is excluding the publishers or consumers from the stats, not including "exclusive" publishers or consumers.
This was my clerical error, done.
--
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
Re: [PR] [improve][pip] PIP-303: Add optional parameters for getPartitionedStats [pulsar]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #21228:
URL: https://github.com/apache/pulsar/pull/21228#issuecomment-1782163882
The pr had no activity for 30 days, mark with Stale label.
--
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] crossoverJie commented on pull request #21228: [improve][pip] PIP-303: Add optional parameters for getPartitionedStats
Posted by "crossoverJie (via GitHub)" <gi...@apache.org>.
crossoverJie commented on PR #21228:
URL: https://github.com/apache/pulsar/pull/21228#issuecomment-1735264054
Great, I'll create a class `GetPartitionedStatsOptions`:
```java
@Data
@Builder
public class GetPartitionedStatsOptions {
/**
* Set to true to get precise backlog, Otherwise get imprecise backlog.
*/
private final boolean getPreciseBacklog;
/**
* Whether to get backlog size for each subscription.
*/
private final boolean subscriptionBacklogSize;
/**
* Whether to get the earliest time in backlog.
*/
private final boolean getEarliestTimeInBacklog;
/**
* Whether to exclusive publishers.
*/
private final boolean exclusivePublishers;
/**
* Whether to exclusive subscriptions.
*/
private final boolean exclusiveSubscriptions;
}
```
---
Refer to this API:
https://github.com/apache/pulsar/blob/82237d3684fe506bcb6426b3b23f413422e6e4fb/pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/GetStatsOptions.java#L26-L41
https://github.com/apache/pulsar/blob/c9fba5f1f7edc6c22219d27af1b7a63ab2d17c44/pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Topics.java#L1136-L1144
If there are no other issues, I'll update the PIP docuement.
--
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] crossoverJie commented on pull request #21228: [improve][pip] PIP-303: Add optional parameters for getPartitionedStats
Posted by "crossoverJie (via GitHub)" <gi...@apache.org>.
crossoverJie commented on PR #21228:
URL: https://github.com/apache/pulsar/pull/21228#issuecomment-1732782087
> More overloads will be added in this way. I'm wondering if we can just pass a `XXXOption` to replace so many parameters?
Good advice, I'll use `GetPartitionedStatsOptions` instead of `exclusivePublishers/exclusiveSubscriptions`.
```java
@Data
@Builder
public class GetPartitionedStatsOptions {
/**
* Set to true to get precise backlog, Otherwise get imprecise backlog.
*/
private final boolean getPreciseBacklog;
/**
* Whether to get backlog size for each subscription.
*/
private final boolean subscriptionBacklogSize;
/**
* Whether to get the earliest time in backlog.
*/
private final boolean getEarliestTimeInBacklog;
}
CompletableFuture<PartitionedTopicStats> getPartitionedStatsAsync(
String topic, boolean perPartition, boolean getPreciseBacklog, boolean subscriptionBacklogSize,
boolean getEarliestTimeInBacklog, GetPartitionedStatsOptions getPartitionedStatsOptions);
```
Any other suggestions?
--
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
Re: [PR] [improve][pip] PIP-303: Add optional parameters for getPartitionedStats [pulsar]
Posted by "crossoverJie (via GitHub)" <gi...@apache.org>.
crossoverJie commented on code in PR #21228:
URL: https://github.com/apache/pulsar/pull/21228#discussion_r1395911890
##########
pip/pip-303.md:
##########
@@ -0,0 +1,202 @@
+
+# Motivation
+
+When a topic has a large number of producers or consumers (over 1k), querying the `pulsarAdmin.topics().getPartitionedStats()` interface is slow and the response size is also large.
+As a result, it's essential to give users the option of querying producer and consumer information.
+
+
+
+# Goals
+
+## In Scope
+
+Add the API for `org.apache.pulsar.client.admin.Topics`
+```java
+CompletableFuture<PartitionedTopicStats> getPartitionedStatsAsync(
+ String topic, boolean perPartition, GetStatsOptions getStatsOptions);
+
+CompletableFuture<TopicStats> getStatsAsync(String topic, GetStatsOptions getStatsOptions);
+```
+
+
+
+## Out of Scope
+
+None.
+
+
+# High Level Design
+
+Implement the `getPartitionedStatsAsync` method, and add the `excludePublishers` and `excludeConsumers` parameters to `{tenant}/{namespace}/{topic}/partitioned-stats` API in `PersistentTopics` and `NonPersistentTopics`.
+
+# Detailed Design
+
+## Design & Implementation Details
+
+
+Add two fields for `org.apache.pulsar.client.admin.GetStatsOptions`
+```java
+@Data
+@Builder
+public class GetStatsOptions {
+ /**
+ * Whether to exclude publishers.
+ */
+ private final boolean excludePublishers;
+
+ /**
+ * Whether to exclude consumers.
+ */
+ private final boolean excludeConsumers;
+
+}
+```
+
+Implement the `getPartitionedStatsAsync` and `getStatsAsync` interface for `org.apache.pulsar.client.admin.internal.TopicsImpl`
+```java
+@Override
+public CompletableFuture<PartitionedTopicStats> getPartitionedStatsAsync(String topic, boolean perPartition, GetStatsOptions getStatsOptions){
+ TopicName tn = validateTopic(topic);
+ WebTarget path = topicPath(tn, "partitioned-stats");
+ path = path.queryParam("perPartition", perPartition)
+ .queryParam("getPreciseBacklog", getStatsOptions.isGetPreciseBacklog())
+ .queryParam("subscriptionBacklogSize", getStatsOptions.isSubscriptionBacklogSize())
+ .queryParam("getEarliestTimeInBacklog", getStatsOptions.isGetEarliestTimeInBacklog());
+ .queryParam("excludePublishers", getStatsOptions.isExcludePublishers())
+ .queryParam("excludeConsumers", getStatsOptions.isExcludeConsumers());
+}
+
+@Override
+public CompletableFuture<TopicStats> getStatsAsync(String topic, GetStatsOptions getStatsOptions){
+ TopicName tn = validateTopic(topic);
+ WebTarget path = topicPath(tn, "stats")
+ .queryParam("getPreciseBacklog", getStatsOptions.isGetPreciseBacklog())
+ .queryParam("subscriptionBacklogSize", getStatsOptions.isSubscriptionBacklogSize())
+ .queryParam("getEarliestTimeInBacklog", getStatsOptions.isGetEarliestTimeInBacklog());
+ .queryParam("excludePublishers",getStatsOptions.isExcludePublishers())
+ .queryParam("excludeConsumers",getStatsOptions.isExcludeConsumers());
+}
+```
+
+Add the `excludePublishers` and `excludeConsumers` parameters to `{tenant}/{namespace}/{topic}/partitioned-stats` API
+```java
+@GET
+@Path("{tenant}/{namespace}/{topic}/partitioned-stats")
+public void getPartitionedStats(
+ @Suspended final AsyncResponse asyncResponse,
+ @ApiParam(value = "Specify the tenant", required = true)
+ @PathParam("tenant") String tenant,
+ @ApiParam(value = "Specify the namespace", required = true)
+ @PathParam("namespace") String namespace,
+ @ApiParam(value = "Specify topic name", required = true)
+ @PathParam("topic") @Encoded String encodedTopic,
+ @ApiParam(value = "Get per partition stats")
+ @QueryParam("perPartition") @DefaultValue("true") boolean perPartition,
+ @ApiParam(value = "Whether leader broker redirected this call to this broker. For internal use.")
+ @QueryParam("authoritative") @DefaultValue("false") boolean authoritative,
+ @ApiParam(value = "If return precise backlog or imprecise backlog")
+ @QueryParam("getPreciseBacklog") @DefaultValue("false") boolean getPreciseBacklog,
+ @ApiParam(value = "If return backlog size for each subscription, require locking on ledger so be careful "
+ + "not to use when there's heavy traffic.")
+ @QueryParam("subscriptionBacklogSize") @DefaultValue("true") boolean subscriptionBacklogSize,
+ @ApiParam(value = "If return the earliest time in backlog")
+ @QueryParam("getEarliestTimeInBacklog") @DefaultValue("false") boolean getEarliestTimeInBacklog,
+ @ApiParam(value = "If exclude the publishers")
+ @QueryParam("excludePublishers") @DefaultValue("false") boolean excludePublishers,
+ @ApiParam(value = "If exclude the consumers")
+ @QueryParam("excludeConsumers") @DefaultValue("false") boolean excludeConsumers)
+
+```
+
+Add the `excludePublishers` and `excludeConsumers` parameters to `{tenant}/{namespace}/{topic}/stats` API
+```java
+@GET
+@Path("{tenant}/{namespace}/{topic}/stats")
+public void getStats(
+ @Suspended final AsyncResponse asyncResponse,
+ @ApiParam(value = "Specify the tenant", required = true)
+ @PathParam("tenant") String tenant,
+ @ApiParam(value = "Specify the namespace", required = true)
+ @PathParam("namespace") String namespace,
+ @ApiParam(value = "Specify topic name", required = true)
+ @PathParam("topic") @Encoded String encodedTopic,
+ @ApiParam(value = "Whether leader broker redirected this call to this broker. For internal use.")
+ @QueryParam("authoritative") @DefaultValue("false") boolean authoritative,
+ @ApiParam(value = "If return precise backlog or imprecise backlog")
+ @QueryParam("getPreciseBacklog") @DefaultValue("false") boolean getPreciseBacklog,
+ @ApiParam(value = "If return backlog size for each subscription, require locking on ledger so be careful "
+ + "not to use when there's heavy traffic.")
+ @QueryParam("subscriptionBacklogSize") @DefaultValue("true") boolean subscriptionBacklogSize,
+ @ApiParam(value = "If return time of the earliest message in backlog")
+ @QueryParam("getEarliestTimeInBacklog") @DefaultValue("false") boolean getEarliestTimeInBacklog,
+ @ApiParam(value = "If exclude the publishers"),
+ @QueryParam("excludePublishers") @DefaultValue("false") boolean excludePublishers,
+ @ApiParam(value = "If exclude the consumers")
+ @QueryParam("excludeConsumers") @DefaultValue("false") boolean excludeConsumers)
+```
+
+
+Add parameters for `org.apache.pulsar.broker.service.Topic`
+```java
+TopicStatsImpl getStats(boolean getPreciseBacklog, boolean subscriptionBacklogSize,
+ boolean getEarliestTimeInBacklog, boolean excludePublishers, boolean excludeConsumers);
+
+CompletableFuture<? extends TopicStatsImpl> asyncGetStats(boolean getPreciseBacklog,
Review Comment:
Good suggestion, done.
--
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
Re: [PR] [improve][pip] PIP-303: Add optional parameters for getPartitionedStats [pulsar]
Posted by "crossoverJie (via GitHub)" <gi...@apache.org>.
crossoverJie commented on code in PR #21228:
URL: https://github.com/apache/pulsar/pull/21228#discussion_r1387443226
##########
pip/pip-303.md:
##########
@@ -0,0 +1,207 @@
+
+# Motivation
+
+When a topic has a large number of producers or consumers (over 1k), querying the `pulsarAdmin.topics().getPartitionedStats()` interface is slow and the response size is also large.
+As a result, it's essential to give users the option of querying producer and consumer information.
+
+
+
+# Goals
+
+## In Scope
+
+Add the API for `org.apache.pulsar.client.admin.Topics`
+```java
+CompletableFuture<PartitionedTopicStats> getPartitionedStatsAsync(
+ String topic, boolean perPartition, GetStatsOptions options);
+
+CompletableFuture<TopicStats> getStatsAsync(String topic, GetStatsOptions options);
+```
+
+
+
+## Out of Scope
+
+None.
+
+
+# High Level Design
+
+Implement the `getPartitionedStatsAsync` method, and add the `exclusivePublishers` and `exclusiveConsumers` parameters to `{tenant}/{namespace}/{topic}/partitioned-stats` API in `PersistentTopics` and `NonPersistentTopics`.
+
+# Detailed Design
+
+## Design & Implementation Details
+
+
+Add two fields for `org.apache.pulsar.client.admin.GetStatsOptions`
+```java
+@Data
+@Builder
+public class GetStatsOptions {
+ /**
+ * Whether to exclusive publishers.
+ */
+ private final boolean exclusivePublishers;
+
+ /**
+ * Whether to exclusive subscriptions.
+ */
+ private final boolean exclusiveConsumers;
+
+}
+```
+
+Implement the `getPartitionedStatsAsync` and `getStatsAsync` interface for `org.apache.pulsar.client.admin.internal.TopicsImpl`
+```java
+@Override
+public CompletableFuture<PartitionedTopicStats> getPartitionedStatsAsync(String topic,
+ boolean perPartition, boolean getPreciseBacklog, boolean subscriptionBacklogSize,boolean getEarliestTimeInBacklog,
+ boolean exclusivePublishers, boolean exclusiveConsumers){
+ TopicName tn=validateTopic(topic);
+ WebTarget path=topicPath(tn,"partitioned-stats");
+ path=path.queryParam("perPartition",perPartition)
+ .queryParam("getPreciseBacklog",getPreciseBacklog)
+ .queryParam("subscriptionBacklogSize",subscriptionBacklogSize)
+ .queryParam("getEarliestTimeInBacklog",getEarliestTimeInBacklog)
+ .queryParam("exclusivePublishers",exclusivePublishers)
+ .queryParam("exclusiveConsumers",exclusiveConsumers);
+}
+
+@Override
+public CompletableFuture<TopicStats> getStatsAsync(String topic, boolean getPreciseBacklog,
+ boolean subscriptionBacklogSize,
+ boolean getEarliestTimeInBacklog,
+ boolean exclusivePublishers, boolean exclusiveConsumers){
+ TopicName tn=validateTopic(topic);
+ WebTarget path=topicPath(tn,"stats")
+ .queryParam("getPreciseBacklog",getPreciseBacklog)
+ .queryParam("subscriptionBacklogSize",subscriptionBacklogSize)
+ .queryParam("getEarliestTimeInBacklog",getEarliestTimeInBacklog).
+ .queryParam("exclusivePublishers",exclusivePublishers)
+ .queryParam("exclusiveConsumers",exclusiveConsumers);
+}
Review Comment:
Thanks for your suggestion, done.
--
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
Re: [PR] [improve][pip] PIP-303: Add optional parameters for getPartitionedStats [pulsar]
Posted by "BewareMyPower (via GitHub)" <gi...@apache.org>.
BewareMyPower commented on code in PR #21228:
URL: https://github.com/apache/pulsar/pull/21228#discussion_r1386695389
##########
pip/pip-303.md:
##########
@@ -0,0 +1,207 @@
+
+# Motivation
+
+When a topic has a large number of producers or consumers (over 1k), querying the `pulsarAdmin.topics().getPartitionedStats()` interface is slow and the response size is also large.
+As a result, it's essential to give users the option of querying producer and consumer information.
+
+
+
+# Goals
+
+## In Scope
+
+Add the API for `org.apache.pulsar.client.admin.Topics`
+```java
+CompletableFuture<PartitionedTopicStats> getPartitionedStatsAsync(
+ String topic, boolean perPartition, GetStatsOptions options);
+
+CompletableFuture<TopicStats> getStatsAsync(String topic, GetStatsOptions options);
+```
+
+
+
+## Out of Scope
+
+None.
+
+
+# High Level Design
+
+Implement the `getPartitionedStatsAsync` method, and add the `exclusivePublishers` and `exclusiveConsumers` parameters to `{tenant}/{namespace}/{topic}/partitioned-stats` API in `PersistentTopics` and `NonPersistentTopics`.
+
+# Detailed Design
+
+## Design & Implementation Details
+
+
+Add two fields for `org.apache.pulsar.client.admin.GetStatsOptions`
+```java
+@Data
+@Builder
+public class GetStatsOptions {
+ /**
+ * Whether to exclusive publishers.
+ */
+ private final boolean exclusivePublishers;
+
+ /**
+ * Whether to exclusive subscriptions.
+ */
+ private final boolean exclusiveConsumers;
+
+}
+```
+
+Implement the `getPartitionedStatsAsync` and `getStatsAsync` interface for `org.apache.pulsar.client.admin.internal.TopicsImpl`
+```java
+@Override
+public CompletableFuture<PartitionedTopicStats> getPartitionedStatsAsync(String topic,
+ boolean perPartition, boolean getPreciseBacklog, boolean subscriptionBacklogSize,boolean getEarliestTimeInBacklog,
+ boolean exclusivePublishers, boolean exclusiveConsumers){
+ TopicName tn=validateTopic(topic);
+ WebTarget path=topicPath(tn,"partitioned-stats");
+ path=path.queryParam("perPartition",perPartition)
+ .queryParam("getPreciseBacklog",getPreciseBacklog)
+ .queryParam("subscriptionBacklogSize",subscriptionBacklogSize)
+ .queryParam("getEarliestTimeInBacklog",getEarliestTimeInBacklog)
+ .queryParam("exclusivePublishers",exclusivePublishers)
+ .queryParam("exclusiveConsumers",exclusiveConsumers);
+}
+
+@Override
+public CompletableFuture<TopicStats> getStatsAsync(String topic, boolean getPreciseBacklog,
+ boolean subscriptionBacklogSize,
+ boolean getEarliestTimeInBacklog,
+ boolean exclusivePublishers, boolean exclusiveConsumers){
+ TopicName tn=validateTopic(topic);
+ WebTarget path=topicPath(tn,"stats")
+ .queryParam("getPreciseBacklog",getPreciseBacklog)
+ .queryParam("subscriptionBacklogSize",subscriptionBacklogSize)
+ .queryParam("getEarliestTimeInBacklog",getEarliestTimeInBacklog).
+ .queryParam("exclusivePublishers",exclusivePublishers)
+ .queryParam("exclusiveConsumers",exclusiveConsumers);
+}
Review Comment:
Could you adjust the indent and add the blanks to make the code more readable?
--
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
Re: [PR] [improve][pip] PIP-303: Add optional parameters for getPartitionedStats [pulsar]
Posted by "nodece (via GitHub)" <gi...@apache.org>.
nodece commented on code in PR #21228:
URL: https://github.com/apache/pulsar/pull/21228#discussion_r1395722267
##########
pip/pip-303.md:
##########
@@ -0,0 +1,202 @@
+
+# Motivation
+
+When a topic has a large number of producers or consumers (over 1k), querying the `pulsarAdmin.topics().getPartitionedStats()` interface is slow and the response size is also large.
+As a result, it's essential to give users the option of querying producer and consumer information.
+
+
+
+# Goals
+
+## In Scope
+
+Add the API for `org.apache.pulsar.client.admin.Topics`
+```java
+CompletableFuture<PartitionedTopicStats> getPartitionedStatsAsync(
+ String topic, boolean perPartition, GetStatsOptions getStatsOptions);
+
+CompletableFuture<TopicStats> getStatsAsync(String topic, GetStatsOptions getStatsOptions);
+```
+
+
+
+## Out of Scope
+
+None.
+
+
+# High Level Design
+
+Implement the `getPartitionedStatsAsync` method, and add the `excludePublishers` and `excludeConsumers` parameters to `{tenant}/{namespace}/{topic}/partitioned-stats` API in `PersistentTopics` and `NonPersistentTopics`.
+
+# Detailed Design
+
+## Design & Implementation Details
+
+
+Add two fields for `org.apache.pulsar.client.admin.GetStatsOptions`
+```java
+@Data
+@Builder
+public class GetStatsOptions {
+ /**
+ * Whether to exclude publishers.
+ */
+ private final boolean excludePublishers;
+
+ /**
+ * Whether to exclude consumers.
+ */
+ private final boolean excludeConsumers;
+
+}
+```
+
+Implement the `getPartitionedStatsAsync` and `getStatsAsync` interface for `org.apache.pulsar.client.admin.internal.TopicsImpl`
+```java
+@Override
+public CompletableFuture<PartitionedTopicStats> getPartitionedStatsAsync(String topic, boolean perPartition, GetStatsOptions getStatsOptions){
+ TopicName tn = validateTopic(topic);
+ WebTarget path = topicPath(tn, "partitioned-stats");
+ path = path.queryParam("perPartition", perPartition)
+ .queryParam("getPreciseBacklog", getStatsOptions.isGetPreciseBacklog())
+ .queryParam("subscriptionBacklogSize", getStatsOptions.isSubscriptionBacklogSize())
+ .queryParam("getEarliestTimeInBacklog", getStatsOptions.isGetEarliestTimeInBacklog());
+ .queryParam("excludePublishers", getStatsOptions.isExcludePublishers())
+ .queryParam("excludeConsumers", getStatsOptions.isExcludeConsumers());
+}
+
+@Override
+public CompletableFuture<TopicStats> getStatsAsync(String topic, GetStatsOptions getStatsOptions){
+ TopicName tn = validateTopic(topic);
+ WebTarget path = topicPath(tn, "stats")
+ .queryParam("getPreciseBacklog", getStatsOptions.isGetPreciseBacklog())
+ .queryParam("subscriptionBacklogSize", getStatsOptions.isSubscriptionBacklogSize())
+ .queryParam("getEarliestTimeInBacklog", getStatsOptions.isGetEarliestTimeInBacklog());
+ .queryParam("excludePublishers",getStatsOptions.isExcludePublishers())
+ .queryParam("excludeConsumers",getStatsOptions.isExcludeConsumers());
+}
+```
+
+Add the `excludePublishers` and `excludeConsumers` parameters to `{tenant}/{namespace}/{topic}/partitioned-stats` API
+```java
+@GET
+@Path("{tenant}/{namespace}/{topic}/partitioned-stats")
+public void getPartitionedStats(
+ @Suspended final AsyncResponse asyncResponse,
+ @ApiParam(value = "Specify the tenant", required = true)
+ @PathParam("tenant") String tenant,
+ @ApiParam(value = "Specify the namespace", required = true)
+ @PathParam("namespace") String namespace,
+ @ApiParam(value = "Specify topic name", required = true)
+ @PathParam("topic") @Encoded String encodedTopic,
+ @ApiParam(value = "Get per partition stats")
+ @QueryParam("perPartition") @DefaultValue("true") boolean perPartition,
+ @ApiParam(value = "Whether leader broker redirected this call to this broker. For internal use.")
+ @QueryParam("authoritative") @DefaultValue("false") boolean authoritative,
+ @ApiParam(value = "If return precise backlog or imprecise backlog")
+ @QueryParam("getPreciseBacklog") @DefaultValue("false") boolean getPreciseBacklog,
+ @ApiParam(value = "If return backlog size for each subscription, require locking on ledger so be careful "
+ + "not to use when there's heavy traffic.")
+ @QueryParam("subscriptionBacklogSize") @DefaultValue("true") boolean subscriptionBacklogSize,
+ @ApiParam(value = "If return the earliest time in backlog")
+ @QueryParam("getEarliestTimeInBacklog") @DefaultValue("false") boolean getEarliestTimeInBacklog,
+ @ApiParam(value = "If exclude the publishers")
+ @QueryParam("excludePublishers") @DefaultValue("false") boolean excludePublishers,
+ @ApiParam(value = "If exclude the consumers")
+ @QueryParam("excludeConsumers") @DefaultValue("false") boolean excludeConsumers)
+
+```
+
+Add the `excludePublishers` and `excludeConsumers` parameters to `{tenant}/{namespace}/{topic}/stats` API
+```java
+@GET
+@Path("{tenant}/{namespace}/{topic}/stats")
+public void getStats(
+ @Suspended final AsyncResponse asyncResponse,
+ @ApiParam(value = "Specify the tenant", required = true)
+ @PathParam("tenant") String tenant,
+ @ApiParam(value = "Specify the namespace", required = true)
+ @PathParam("namespace") String namespace,
+ @ApiParam(value = "Specify topic name", required = true)
+ @PathParam("topic") @Encoded String encodedTopic,
+ @ApiParam(value = "Whether leader broker redirected this call to this broker. For internal use.")
+ @QueryParam("authoritative") @DefaultValue("false") boolean authoritative,
+ @ApiParam(value = "If return precise backlog or imprecise backlog")
+ @QueryParam("getPreciseBacklog") @DefaultValue("false") boolean getPreciseBacklog,
+ @ApiParam(value = "If return backlog size for each subscription, require locking on ledger so be careful "
+ + "not to use when there's heavy traffic.")
+ @QueryParam("subscriptionBacklogSize") @DefaultValue("true") boolean subscriptionBacklogSize,
+ @ApiParam(value = "If return time of the earliest message in backlog")
+ @QueryParam("getEarliestTimeInBacklog") @DefaultValue("false") boolean getEarliestTimeInBacklog,
+ @ApiParam(value = "If exclude the publishers"),
+ @QueryParam("excludePublishers") @DefaultValue("false") boolean excludePublishers,
+ @ApiParam(value = "If exclude the consumers")
+ @QueryParam("excludeConsumers") @DefaultValue("false") boolean excludeConsumers)
+```
+
+
+Add parameters for `org.apache.pulsar.broker.service.Topic`
+```java
+TopicStatsImpl getStats(boolean getPreciseBacklog, boolean subscriptionBacklogSize,
+ boolean getEarliestTimeInBacklog, boolean excludePublishers, boolean excludeConsumers);
+
+CompletableFuture<? extends TopicStatsImpl> asyncGetStats(boolean getPreciseBacklog,
Review Comment:
IMO, we only need to add an async method to avoid blocking the thread.
For the method arguments, we can also use the above design, moving these arguments to a class.
--
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] BewareMyPower commented on pull request #21228: [improve][pip] PIP-303: Add optional parameters for getPartitionedStats
Posted by "BewareMyPower (via GitHub)" <gi...@apache.org>.
BewareMyPower commented on PR #21228:
URL: https://github.com/apache/pulsar/pull/21228#issuecomment-1732800205
I think we can package all parameters other than `topic` and `perPartition`, which are required. You can see the following overload:
```java
default PartitionedTopicStats getPartitionedStats(String topic, boolean perPartition) throws PulsarAdminException {
return getPartitionedStats(topic, perPartition, false, false, false);
}
```
--
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