eolivelli commented on code in PR #20179:
URL: https://github.com/apache/pulsar/pull/20179#discussion_r1189722242
##########
pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/SubscriptionStats.java:
##########
@@ -115,8 +115,8 @@ public interface SubscriptionStats {
/** Whether the Key_Shared subscription mode is AUTO_SPLIT or STICKY. */
String getKeySharedMode();
- /** This is for Key_Shared subscription to get the recentJoinedConsumers
in the Key_Shared subscription. */
- Map<String, String> getConsumersAfterMarkDeletePosition();
+ /** This is for Key_Shared subscription to get the recentlyJoinedConsumers
in the Key_Shared subscription. */
Review Comment:
the same here
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java:
##########
@@ -1186,11 +1186,19 @@ public SubscriptionStatsImpl getStats(Boolean
getPreciseBacklog, boolean subscri
subStats.allowOutOfOrderDelivery =
keySharedDispatcher.isAllowOutOfOrderDelivery();
subStats.keySharedMode =
keySharedDispatcher.getKeySharedMode().toString();
- LinkedHashMap<Consumer, PositionImpl> recentlyJoinedConsumers =
keySharedDispatcher
+ LinkedHashMap<Consumer,
PersistentStickyKeyDispatcherMultipleConsumers.LastSentPositions>
Review Comment:
this is a code smell to refer to this implementation class
PersistentStickyKeyDispatcherMultipleConsumers, maybe we should move the
LastSentPositions to a top level public class
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java:
##########
@@ -1186,11 +1186,19 @@ public SubscriptionStatsImpl getStats(Boolean
getPreciseBacklog, boolean subscri
subStats.allowOutOfOrderDelivery =
keySharedDispatcher.isAllowOutOfOrderDelivery();
subStats.keySharedMode =
keySharedDispatcher.getKeySharedMode().toString();
- LinkedHashMap<Consumer, PositionImpl> recentlyJoinedConsumers =
keySharedDispatcher
+ LinkedHashMap<Consumer,
PersistentStickyKeyDispatcherMultipleConsumers.LastSentPositions>
+ recentlyJoinedConsumers = keySharedDispatcher
.getRecentlyJoinedConsumers();
if (recentlyJoinedConsumers != null &&
recentlyJoinedConsumers.size() > 0) {
recentlyJoinedConsumers.forEach((k, v) -> {
-
subStats.consumersAfterMarkDeletePosition.put(k.consumerName(), v.toString());
+ // Dispatchers allows same name consumers
+ final StringBuilder stringBuilder = new StringBuilder();
Review Comment:
could we use a struct instead of a string ? (you can use a 'record' now, but
as this fix should be cherrypick a plain old java object works better)
strings are error prone in code and also less efficient
##########
pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/ConsumerStats.java:
##########
@@ -69,8 +69,8 @@ public interface ConsumerStats {
/** Flag to verify if consumer is blocked due to reaching threshold of
unacked messages. */
boolean isBlockedConsumerOnUnackedMsgs();
- /** The read position of the cursor when the consumer joining. */
- String getReadPositionWhenJoining();
+ /** Last sent positions per sticky key of the cursor when the consumer
joining. */
Review Comment:
This is a breaking API change, please do not do it
you can add the new method but we should keep the old value as well,
otherwise we cannot cherry-pick to old branches
--
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-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
