[improve][pip] PIP-303: Add optional parameters for getPartitionedStats

[crossoverJie]

Releted issue: #21200

Documentation

• doc
• doc-required
• doc-not-needed
• doc-complete

[BewareMyPower]

More overloads will be added in this way. I'm wondering if we can just pass a XXXOption to replace so many parameters?

[crossoverJie]

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.

@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?

[BewareMyPower]

I think we can package all parameters other than topic and perPartition, which are required. You can see the following overload:

    default PartitionedTopicStats getPartitionedStats(String topic, boolean perPartition) throws PulsarAdminException {
        return getPartitionedStats(topic, perPartition, false, false, false);
    }

[crossoverJie]

Great, I'll create a class GetPartitionedStatsOptions:

@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:

pulsar/pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/GetStatsOptions.java

Lines 26 to 41 in 82237d3

	public class GetStatsOptions {
	/**
	* 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;
	}

pulsar/pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Topics.java

Lines 1136 to 1144 in c9fba5f

	TopicStats getStats(String topic, GetStatsOptions getStatsOptions) throws PulsarAdminException;
	default TopicStats getStats(String topic, boolean getPreciseBacklog, boolean subscriptionBacklogSize,
	boolean getEarliestTimeInBacklog) throws PulsarAdminException {
	GetStatsOptions getStatsOptions =
	new GetStatsOptions(getPreciseBacklog, subscriptionBacklogSize, getEarliestTimeInBacklog);
	return getStats(topic, getStatsOptions);
	}

If there are no other issues, I'll update the PIP docuement.

[github-actions]

The pr had no activity for 30 days, mark with Stale label.

[crossoverJie]

@BewareMyPower I have updated the PIP, please take a look.

[BewareMyPower]

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.

[crossoverJie]

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.

[nodece]

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.

[crossoverJie]

Good suggestion, done.

[nodece]

I left a suggestion, can you confirm?

LGTM