MarvinCai commented on a change in pull request #9302:
URL: https://github.com/apache/pulsar/pull/9302#discussion_r564123990



##########
File path: 
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -1826,6 +1826,13 @@
     )
     private boolean exposePreciseBacklogInPrometheus = false;
 
+    @FieldContext(
+            category = CATEGORY_METRICS,
+            doc = "Enable expose the backlog size fro each subscription when 
generating stats.\n" +
+                    " Locking is used for fetching the status so default to 
false."
+    )
+    private boolean exposeSubscriptionBacklokSizeInPrometheus = false;

Review comment:
       @codelipenghui 
   I think it is actually quiet expensive, it's using existing 
[estimateBacklogFromPosition(PositionImpl 
pos)](https://github.com/apache/pulsar/blob/master/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java#L1120)
 to get the backlog size, which requires locking the whole MLedger object. And 
this will happen for every subscription, periodic metric generation with this 
subscription backlog size could affect performance and could potentially 
compete with the ledger trimming task, so I think we should add a flag for it 
and disable by default.




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to