cshannon commented on code in PR #1484:
URL: https://github.com/apache/activemq/pull/1484#discussion_r3895849790
##########
activemq-broker/src/main/java/org/apache/activemq/broker/region/BaseDestination.java:
##########
@@ -311,12 +313,46 @@ 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.
+ // Durable subscriptions never permit gc - their registration and
pending messages live in the
+ // destination's store, which gc destroys, so collecting would break
the durability guarantee.
+ return canGcNetwork || (isGcWithOnlyWildcardConsumers() &&
subscription.isWildcard()
Review Comment:
Why did you make this specific change with durables? I don't think this
change is necessary and it also isn't described anywhere and is outside of the
scope of the original PR.
Destinations get marked for GC only if the pending size is 0 and there are
no producers/consumers. If keepDurableSubsActive = false and there are no
pending messages anyways, why would that block GC? If you set
keepDurableSubsActive = true then it would count as a consumer and stay active.
I could see a use case for the existence of any durable blocking the removal
of a destination, but i don't think that check would fall under the "is active"
check. While a bit annoying, this feels like yet another policy setting that is
something like "preventDestinationGCWithDurables" or something like that and
should likely be an entirely separate PR.
--
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
