cshannon commented on code in PR #1484:
URL: https://github.com/apache/activemq/pull/1484#discussion_r3899469748
##########
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:
@mattrpav - I think we just remove all the special durable handling entirely
as it's really out of the scope of this issue and PR.
I don't think we need to actually do anything special for durables because
there's already a mechanism for it with the `keepDurableSubsActive` flag that
can be used to prevent GC'ing if desired. The current check for active
destinations doesn't care specifically about durables, just consumers. If that
flag is true then a durable is considered an online consumer and we don't GC,
if false it's treated just like any other offline consumer and we can GC if no
pending messages exist.
So, I would remove all the durable stuff and if you have a use case to
change the behavior around how durable subscriptions work in conjunction with
the GC active check, I would make that an entirely separate issue/PR and also
it would need to take into account all use cases, not just wild cards. (but
like i said it seems entirely uneeded)
--
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
