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/08/30 10:13:40 UTC

[GitHub] [pulsar] massakam opened a new pull request #11839: [stats] Add Key_Shared metadata to topic stats

massakam opened a new pull request #11839:
URL: https://github.com/apache/pulsar/pull/11839


   ### Motivation
   
   Users can specify the Key_Shared subscription policy as follows:
   ```java
   Consumer<byte[]> consumer = pulsarClient.newConsumer()
           .topic("persistent://public/default/t1")
           .subscriptionName("sub1")
           .subscriptionType(SubscriptionType.Key_Shared)
           .keySharedPolicy(KeySharedPolicy.autoSplitHashRange().setAllowOutOfOrderDelivery(true))
           .subscribe();
   ```
   
   However, these metadata are not currently included in the topic stats.
   
   ### Modifications
   
   Added the following two fields to the topic stats:
   - allowOutOfOrderDelivery
   - keySharedMode
   
   ### Verifying this change
   
   - [ ] Make sure that the change passes the CI checks.
   
   ### 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)


-- 
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] Anonymitaet commented on pull request #11839: [stats] Add Key_Shared metadata to topic stats

Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on pull request #11839:
URL: https://github.com/apache/pulsar/pull/11839#issuecomment-908823557


   @massakam thanks for adding docs!


-- 
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] Anonymitaet commented on pull request #11839: [stats] Add Key_Shared metadata to topic stats

Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on pull request #11839:
URL: https://github.com/apache/pulsar/pull/11839#issuecomment-908823557


   @massakam thanks for adding docs!


-- 
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] massakam commented on pull request #11839: [stats] Add Key_Shared metadata to topic stats

Posted by GitBox <gi...@apache.org>.
massakam commented on pull request #11839:
URL: https://github.com/apache/pulsar/pull/11839#issuecomment-910160486


   PTAL


-- 
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] massakam commented on a change in pull request #11839: [stats] Add Key_Shared metadata to topic stats

Posted by GitBox <gi...@apache.org>.
massakam commented on a change in pull request #11839:
URL: https://github.com/apache/pulsar/pull/11839#discussion_r699412556



##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/SubscriptionStatsImpl.java
##########
@@ -103,6 +103,12 @@
     /** Mark that the subscription state is kept in sync across different regions. */
     public boolean isReplicated;
 
+    /** Whether out of order delivery is allowed on the Key_Shared subscription. */
+    public boolean allowOutOfOrderDelivery;

Review comment:
       I actually added the above sub-interface/sub-class, but then the deserialization of the json response didn't seem to work on the pulsar-client-admin side. So I just added getters to the `SubscriptionStats` interface.




-- 
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] massakam commented on a change in pull request #11839: [stats] Add Key_Shared metadata to topic stats

Posted by GitBox <gi...@apache.org>.
massakam commented on a change in pull request #11839:
URL: https://github.com/apache/pulsar/pull/11839#discussion_r699412556



##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/SubscriptionStatsImpl.java
##########
@@ -103,6 +103,12 @@
     /** Mark that the subscription state is kept in sync across different regions. */
     public boolean isReplicated;
 
+    /** Whether out of order delivery is allowed on the Key_Shared subscription. */
+    public boolean allowOutOfOrderDelivery;

Review comment:
       I actually added the above sub-interface/sub-class, but then the deserialization of the json response didn't seem to work on the pulsar-client-admin side. So I just added getters to the `SubscriptionStats` interface.




-- 
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] massakam commented on a change in pull request #11839: [stats] Add Key_Shared metadata to topic stats

Posted by GitBox <gi...@apache.org>.
massakam commented on a change in pull request #11839:
URL: https://github.com/apache/pulsar/pull/11839#discussion_r698946321



##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/SubscriptionStatsImpl.java
##########
@@ -103,6 +103,12 @@
     /** Mark that the subscription state is kept in sync across different regions. */
     public boolean isReplicated;
 
+    /** Whether out of order delivery is allowed on the Key_Shared subscription. */
+    public boolean allowOutOfOrderDelivery;

Review comment:
       @merlimat 
   > Also, since this is specific to KeyShared subscriptions, it might get confusing when using other subscription types. We could instead introduce a sub-interface/sub-object to show the key-shared policy.
   
   Does that mean we should add the following two?
   
   - `KeySharedSubscriptionStats` interface that extends the `SubscriptionStats` interface
   - `KeySharedSubscriptionStatsImpl` class that extends the `SubscriptionStatsImpl` class and implements the `KeySharedSubscriptionStats` interface




-- 
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] merlimat commented on a change in pull request #11839: [stats] Add Key_Shared metadata to topic stats

Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #11839:
URL: https://github.com/apache/pulsar/pull/11839#discussion_r698609295



##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/SubscriptionStatsImpl.java
##########
@@ -103,6 +103,12 @@
     /** Mark that the subscription state is kept in sync across different regions. */
     public boolean isReplicated;
 
+    /** Whether out of order delivery is allowed on the Key_Shared subscription. */
+    public boolean allowOutOfOrderDelivery;

Review comment:
       This should also be added to the `SubscriptionStats` interface. 
   
   Also, since this is specific to KeyShared subscriptions, it might get confusing when using other subscription types. We could instead introduce a sub-interface/sub-object to show the key-shared policy. 




-- 
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] massakam commented on a change in pull request #11839: [stats] Add Key_Shared metadata to topic stats

Posted by GitBox <gi...@apache.org>.
massakam commented on a change in pull request #11839:
URL: https://github.com/apache/pulsar/pull/11839#discussion_r698946321



##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/SubscriptionStatsImpl.java
##########
@@ -103,6 +103,12 @@
     /** Mark that the subscription state is kept in sync across different regions. */
     public boolean isReplicated;
 
+    /** Whether out of order delivery is allowed on the Key_Shared subscription. */
+    public boolean allowOutOfOrderDelivery;

Review comment:
       @merlimat 
   > Also, since this is specific to KeyShared subscriptions, it might get confusing when using other subscription types. We could instead introduce a sub-interface/sub-object to show the key-shared policy.
   
   Does that mean we should add the following two?
   
   - `KeySharedSubscriptionStats` interface that extends the `SubscriptionStats` interface
   - `KeySharedSubscriptionStatsImpl` class that extends the `SubscriptionStatsImpl` class and implements the `KeySharedSubscriptionStats` interface




-- 
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] merlimat commented on a change in pull request #11839: [stats] Add Key_Shared metadata to topic stats

Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #11839:
URL: https://github.com/apache/pulsar/pull/11839#discussion_r698609295



##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/SubscriptionStatsImpl.java
##########
@@ -103,6 +103,12 @@
     /** Mark that the subscription state is kept in sync across different regions. */
     public boolean isReplicated;
 
+    /** Whether out of order delivery is allowed on the Key_Shared subscription. */
+    public boolean allowOutOfOrderDelivery;

Review comment:
       This should also be added to the `SubscriptionStats` interface. 
   
   Also, since this is specific to KeyShared subscriptions, it might get confusing when using other subscription types. We could instead introduce a sub-interface/sub-object to show the key-shared policy. 




-- 
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] nkurihar merged pull request #11839: [stats] Add Key_Shared metadata to topic stats

Posted by GitBox <gi...@apache.org>.
nkurihar merged pull request #11839:
URL: https://github.com/apache/pulsar/pull/11839


   


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