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/02/11 18:12:45 UTC

[GitHub] [pulsar] rdhabalia opened a new pull request #14251: [pulsar-broker] provide readable consumer stats time

rdhabalia opened a new pull request #14251:
URL: https://github.com/apache/pulsar/pull/14251


   ### Motivation
   Pulsar topic stats shows `connectedSince` time in topics-stats-CLI per consumer in human-readable format which helps user to troubleshoot issue when large number of consumers are connected. similar way it's also helpful if topic-stats shows `lastAckedTime`  and `lastConsumedTime` into readable format instead timestamp to make it easier for user/admin for troubleshoot issue for specific consumer when large number of consumers are connected on subscription. So, deprecating timestamp and adding readable time format for `lastAckedTime`  and `lastConsumedTime` , and also update the document accordingly. 
   
   ### Result
   User will be able to see `lastAckedTime`  and `lastConsumedTime` in readable format.
   ```
   "lastConsumedTime" : "2022-02-11T09:33:08.152-08:00",
   "lastAckedTime" : "2022-02-11T09:33:08.181-08:00",
   ```


-- 
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] rdhabalia commented on a change in pull request #14251: [pulsar-broker] provide readable consumer stats time

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



##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/ConsumerStatsImpl.java
##########
@@ -84,7 +85,11 @@
     @JsonIgnore
     private int clientVersionLength;
 
+    // ignore this json field to skip from stats in future release. replaced with readable #getLastAckedTime().
+    @Deprecated

Review comment:
       I think if app requires a calculation then app can always programmatically changes date to timestamp.

##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/ConsumerStatsImpl.java
##########
@@ -84,7 +85,11 @@
     @JsonIgnore
     private int clientVersionLength;
 
+    // ignore this json field to skip from stats in future release. replaced with readable #getLastAckedTime().
+    @Deprecated

Review comment:
       I think if app requires a calculation then app can always programmatically change date to timestamp.




-- 
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] Jason918 commented on a change in pull request #14251: [pulsar-broker] provide readable consumer stats time

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



##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/ConsumerStatsImpl.java
##########
@@ -84,7 +85,11 @@
     @JsonIgnore
     private int clientVersionLength;
 
+    // ignore this json field to skip from stats in future release. replaced with readable #getLastAckedTime().
+    @Deprecated

Review comment:
       Is it ok to keep the timestamp? 
   It's useful to calculate time used, like how long since it's last acked.




-- 
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] github-actions[bot] commented on pull request #14251: [pulsar-broker] provide readable consumer stats time

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14251:
URL: https://github.com/apache/pulsar/pull/14251#issuecomment-1036485095


   @rdhabalia:Thanks for your contribution. For this PR, do we need to update docs?
   (The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)


-- 
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] eolivelli commented on pull request #14251: [pulsar-broker] provide readable consumer stats time

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


   we should probably rebase and see that CI passes, then we can merge
   
   thank you @rdhabalia , sorry for being late on merging this patch


-- 
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] github-actions[bot] commented on pull request #14251: [pulsar-broker] provide readable consumer stats time

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14251:
URL: https://github.com/apache/pulsar/pull/14251#issuecomment-1069832926


   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] eolivelli commented on pull request #14251: [pulsar-broker] provide readable consumer stats time

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


   we should probably rebase and see that CI passes, then we can merge
   
   thank you @rdhabalia , sorry for being late on merging this patch


-- 
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] rdhabalia commented on a change in pull request #14251: [pulsar-broker] provide readable consumer stats time

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



##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/ConsumerStatsImpl.java
##########
@@ -84,7 +85,11 @@
     @JsonIgnore
     private int clientVersionLength;
 
+    // ignore this json field to skip from stats in future release. replaced with readable #getLastAckedTime().
+    @Deprecated

Review comment:
       I think if app requires a calculation then app can always programmatically changes date to timestamp.

##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/ConsumerStatsImpl.java
##########
@@ -84,7 +85,11 @@
     @JsonIgnore
     private int clientVersionLength;
 
+    // ignore this json field to skip from stats in future release. replaced with readable #getLastAckedTime().
+    @Deprecated

Review comment:
       I think if app requires a calculation then app can always programmatically change date to timestamp.




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