mattrpav commented on code in PR #1288:
URL: https://github.com/apache/activemq/pull/1288#discussion_r1813211735


##########
activemq-broker/src/main/java/org/apache/activemq/broker/jmx/BrokerView.java:
##########
@@ -280,11 +280,21 @@ public ObjectName[] getTopics() {
         return safeGetBroker().getTopicsNonSuppressed();
     }
 
+    @Override
+    public int getTotalTopicsCount() {
+        return getTopics().length;
+    }
+
     @Override
     public ObjectName[] getQueues() {
         return safeGetBroker().getQueuesNonSuppressed();
     }
 
+    @Override
+    public int getTotalQueuesCount() {
+        return getQueues().length;

Review Comment:
   My proposal is _way_ more useful for users that _use_ suppression, since 
they will have a way to know what the actual total number of queues in the 
broker actually is. When you use suppression with the current approach, there 
is information loss and it isn't useful. The purpose of suppression is to 
reduce performance impact of registering JMX beans for objects that are 
added/removed quickly.
   
   For mixed (some suppressed, some not) use cases, this is super valuable.
   
      queue://order.>  <-- namespace not suppressed 
      queue://one.time.queues.>  <-- suppressed, since they are added/removed 
fast
   
   Users would _need_ to know how many total queues are _actually_ in the 
broker, not just the number of queue://order.>, since that is already available 
in the list of registered queues.
   
   Use cases:
   
   Use Case 1: 128 queues, no jmx mbean suppression
   Use Case 2: 128 queues, 128 queues suppressed
   Use Case 3: 128 queues, 28 queues suppressed
   
   Current proposal:
   
   BrokerView is defined as what is suppressed/registered with JMX
   
   | Use Case | # queues | # suppressed | getQueues() result | 
getTotalQueueCount() |
   | ------------- | ------------- | ------------- | ------------- | 
------------- |
   | 1 | 128 | 0 | ObjectName[128] | 128 |
   | 2 | 128 | 128 | ObjectName[0] | 0 |
   | 3 | 128 | 28 | ObjectName[100] | 100 |
   
   mattrpav proposal:
   
   BrokerView is defined as what is in the broker regardless of suppression
   
   | Use Case | # queues | # suppressed | getQueues() result | 
getTotalQueueCount() |
   | ------------- | ------------- | ------------- | ------------- | 
------------- |
   | 1 | 128 | 0 | ObjectName[128] | 128 |
   | 2 | 128 | 128 | ObjectName[0] | 128 |
   | 3 | 128 | 28 | ObjectName[100] | 128 |
   
   
   getQueues() <-- returns ObjectName[], an array of JMX ObjectNames for queues 
that are registered, this isn't _all queues in the broker_. If anything, this 
method is mislabeled.
   
   The BrokerView shows connection counts as _actuals_, and not what is 
_registered_.



-- 
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: gitbox-unsubscr...@activemq.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact


Reply via email to