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


##########
activemq-broker/src/main/java/org/apache/activemq/broker/region/BaseDestination.java:
##########
@@ -311,12 +313,42 @@ public final MessageStore getMessageStore() {
 
     @Override
     public boolean isActive() {
-        boolean isActive = destinationStatistics.getConsumers().getCount() > 0 
||
-                           destinationStatistics.getProducers().getCount() > 0;
-        if (isActive && isGcWithNetworkConsumers() && 
destinationStatistics.getConsumers().getCount() > 0) {
-            isActive = hasRegularConsumers(getConsumers());
+        // if we have producers then we are active
+        if (destinationStatistics.getProducers().getCount() > 0) {
+            return true;
         }
-        return isActive;
+
+        // Check if we have active consumers that should prevent GC
+        if (destinationStatistics.getConsumers().getCount() > 0) {
+            // if we have consumers and both gcWithNetwork and gcOnlyWildcard 
consumers
+            // are false we can just return true, otherwise we need to check 
each consumer
+            return (!isGcWithNetworkConsumers() && 
!isGcWithOnlyWildcardConsumers()) ||
+                    hasActiveConsumers();
+        }
+
+        return false;
+    }
+
+    protected Predicate<Subscription> canGcConsumer = subscription -> {
+        // if isGcWithNetworkConsumers() is true and this is a network 
subscription then we can GC
+        boolean canGcNetwork = isGcWithNetworkConsumers() && 
subscription.getConsumerInfo().isNetworkSubscription();
+        // if isGcWithOnlyWildcardConsumers() is true and this is a 
non-durable wildcard then we can GC.
+        // An attached durable subscription never permits gc - its 
registration and pending messages
+        // live in the destination's store, which gc destroys. Note offline 
durable subscriptions
+        // stay attached only with keepDurableSubsActive=true (the default); 
brokers running with
+        // keepDurableSubsActive=false forfeit this protection while the 
subscriber is offline.
+        return canGcNetwork || (isGcWithOnlyWildcardConsumers() && 
subscription.isWildcard()
+                && !subscription.getConsumerInfo().isDurable());

Review Comment:
   I was able to remove the counter comparison check; however, if this 
.isDurable() check is removed a topic with only one wildcard durable 
subscription would be deleted and risk message loss.



-- 
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: [email protected]

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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact


Reply via email to