cshannon commented on code in PR #1484:
URL: https://github.com/apache/activemq/pull/1484#discussion_r3908819321
##########
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:
@mattrpav - what about this, instead of checking if any subs are durables,
we could do the following and check sub pending size for all subs, which is
what we really care about (are there pending messages):
```java
protected Predicate<Subscription> canGcConsumer = subscription -> {
// Check 1: if isGcWithNetworkConsumers() is true and this is a network
subscription then we can GC
boolean canGcNetwork = isGcWithNetworkConsumers() &&
subscription.getConsumerInfo().isNetworkSubscription();
// Check 2: if isGcWithOnlyWildcardConsumers() is true and this is a
wildcard then we can GC.
//
// Check 3: if the pending size is > 0, then we can't' GC because there
are pending messages
// This check is necessary for subs on a Topic in particular because the
messages count metric
// used inside of canGC() won't track the messages after they are passed
to the subs.
//
// Note: getPendingQueueSize() will always return 0 for on offline
durable sub unless
// the keepDurableSubsActive flag is set to true. This means that
offline durables that
// have keepDurableSubsActive=false will not block GC even if there are
pending messages.
return canGcNetwork || (isGcWithOnlyWildcardConsumers() &&
subscription.isWildcard())
|| subscription.getPendingQueueSize() == 0;
};
```
The obvious glaring thing is how to handle the case of offline durables
where keepDurableSubsActive=false. The above keeps the current behavior and
makes them eligible for GC. I am hesitant to make yet another flag, but I
suppose we could as a separate task to allow the existence of offline durables,
even if keepDurableSubsActive=false, to prevent GC.
--
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
