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:
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.
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 | 128 ObjectName[] | 128 |
| 2 | 128 | 128 | 0 ObjectName[] | 0 |
| 3 | 128 | 28 | 100 ObjectName[] | 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 | 128 ObjectName[] | 128 |
| 2 | 128 | 128 | 0 ObjectName[] | 128 |
| 3 | 128 | 28 | 100 ObjectName[] | 128 |
--
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
