You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by jbertram <gi...@git.apache.org> on 2018/10/23 15:33:11 UTC
[GitHub] activemq-artemis pull request #2388: ARTEMIS-1856 support delays before dele...
GitHub user jbertram opened a pull request:
https://github.com/apache/activemq-artemis/pull/2388
ARTEMIS-1856 support delays before deleting addresses & queues
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jbertram/activemq-artemis ARTEMIS-1856
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/2388.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2388
----
commit e6b099db88317f8eb2913a48d6e8d53fc0e5ea8e
Author: Justin Bertram <jb...@...>
Date: 2018-09-24T21:37:28Z
ARTEMIS-1856 support delays before deleting addresses & queues
----
---
[GitHub] activemq-artemis issue #2388: ARTEMIS-1856 support delays before deleting ad...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/2388
@jbertram it wasn't a biggie, i guess more important things are a foot, so merging. And can always add the extra config to the other as and when needed in future.
---
[GitHub] activemq-artemis issue #2388: ARTEMIS-1856 support delays before deleting ad...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/2388
LGTM, will merge tomorrow if no one beats me to it.
---
[GitHub] activemq-artemis pull request #2388: ARTEMIS-1856 support delays before dele...
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2388#discussion_r227609075
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java ---
@@ -1540,6 +1561,75 @@ public void run() {
}
}
+ private final class AddressQueueReaper extends ActiveMQScheduledComponent {
+
+ AddressQueueReaper(ScheduledExecutorService scheduledExecutorService,
+ Executor executor,
+ long checkPeriod,
+ TimeUnit timeUnit,
+ boolean onDemand) {
+ super(scheduledExecutorService, executor, checkPeriod, timeUnit, onDemand);
+ }
+
+ @Override
+ public void run() {
+ Map<SimpleString, Binding> nameMap = addressManager.getBindings();
+
+ List<Queue> queues = new ArrayList<>();
+
+ for (Binding binding : nameMap.values()) {
+ if (binding.getType() == BindingType.LOCAL_QUEUE) {
+ Queue queue = (Queue) binding.getBindable();
+
+ queues.add(queue);
+ }
+ }
+
+ for (Queue queue : queues) {
+ int consumerCount = queue.getConsumerCount();
+ long messageCount = queue.getMessageCount();
+ boolean autoCreated = queue.isAutoCreated();
+ long consumerRemovedTimestamp = queue.getConsumerRemovedTimestamp();
+
+ if (!queue.isInternalQueue() && autoCreated && messageCount == 0 && consumerCount == 0 && consumerRemovedTimestamp != -1) {
+ SimpleString queueName = queue.getName();
+ AddressSettings settings = addressSettingsRepository.getMatch(queue.getAddress().toString());
+ if (settings.isAutoDeleteQueues() && (System.currentTimeMillis() - consumerRemovedTimestamp >= settings.getAutoDeleteQueuesDelay())) {
+ if (ActiveMQServerLogger.LOGGER.isDebugEnabled()) {
+ ActiveMQServerLogger.LOGGER.info("deleting auto-created queue \"" + queueName + ".\" consumerCount = " + consumerCount + "; messageCount = " + messageCount + "; isAutoDeleteQueues = " + settings.isAutoDeleteQueues());
+ }
+
+ try {
+ server.destroyQueue(queueName, null, true, false);
--- End diff --
@michaelandrepearce is right, but I think it's worth mitigating the risk within the bounds of the current locking scheme. What I don't want to do is introduce additional locking or synchronization.
---
[GitHub] activemq-artemis issue #2388: ARTEMIS-1856 support delays before deleting ad...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/2388
@jbertram went to merge this morning and during the process and noticed something, have added comment inline
---
[GitHub] activemq-artemis pull request #2388: ARTEMIS-1856 support delays before dele...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2388#discussion_r227665421
--- Diff: artemis-server/src/main/resources/schema/artemis-configuration.xsd ---
@@ -2953,6 +2961,15 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="auto-delete-queues-delay" type="xsd:long" default="0" maxOccurs="1" minOccurs="0">
--- End diff --
ditto - Can this be added to the artemis-configuration used for testing to keep as much as possible the two in sync
---
[GitHub] activemq-artemis pull request #2388: ARTEMIS-1856 support delays before dele...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/2388
---
[GitHub] activemq-artemis pull request #2388: ARTEMIS-1856 support delays before dele...
Posted by sebthom <gi...@git.apache.org>.
Github user sebthom commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2388#discussion_r227494586
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java ---
@@ -1540,6 +1561,75 @@ public void run() {
}
}
+ private final class AddressQueueReaper extends ActiveMQScheduledComponent {
+
+ AddressQueueReaper(ScheduledExecutorService scheduledExecutorService,
+ Executor executor,
+ long checkPeriod,
+ TimeUnit timeUnit,
+ boolean onDemand) {
+ super(scheduledExecutorService, executor, checkPeriod, timeUnit, onDemand);
+ }
+
+ @Override
+ public void run() {
+ Map<SimpleString, Binding> nameMap = addressManager.getBindings();
+
+ List<Queue> queues = new ArrayList<>();
+
+ for (Binding binding : nameMap.values()) {
+ if (binding.getType() == BindingType.LOCAL_QUEUE) {
+ Queue queue = (Queue) binding.getBindable();
+
+ queues.add(queue);
+ }
+ }
+
+ for (Queue queue : queues) {
+ int consumerCount = queue.getConsumerCount();
+ long messageCount = queue.getMessageCount();
+ boolean autoCreated = queue.isAutoCreated();
+ long consumerRemovedTimestamp = queue.getConsumerRemovedTimestamp();
+
+ if (!queue.isInternalQueue() && autoCreated && messageCount == 0 && consumerCount == 0 && consumerRemovedTimestamp != -1) {
+ SimpleString queueName = queue.getName();
+ AddressSettings settings = addressSettingsRepository.getMatch(queue.getAddress().toString());
+ if (settings.isAutoDeleteQueues() && (System.currentTimeMillis() - consumerRemovedTimestamp >= settings.getAutoDeleteQueuesDelay())) {
+ if (ActiveMQServerLogger.LOGGER.isDebugEnabled()) {
+ ActiveMQServerLogger.LOGGER.info("deleting auto-created queue \"" + queueName + ".\" consumerCount = " + consumerCount + "; messageCount = " + messageCount + "; isAutoDeleteQueues = " + settings.isAutoDeleteQueues());
+ }
+
+ try {
+ server.destroyQueue(queueName, null, true, false);
--- End diff --
What happens if a message was added to the queue in the time between queue.getMessageCount() was invoked and server.destroyQueue() is executed. Maybe ActiveMQServer.destroyQueue() needs to be extended by a "checkMessageCount" parameter?
---
[GitHub] activemq-artemis pull request #2388: ARTEMIS-1856 support delays before dele...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2388#discussion_r227665143
--- Diff: artemis-server/src/main/resources/schema/artemis-configuration.xsd ---
@@ -336,6 +336,14 @@
</xsd:annotation>
</xsd:element>
+ <xsd:element name="address-queue-scan-period" type="xsd:long" default="30000" maxOccurs="1" minOccurs="0">
--- End diff --
Can this be added to the artemis-configuration used for testing to keep as much as possible the two in sync
---
[GitHub] activemq-artemis pull request #2388: ARTEMIS-1856 support delays before dele...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2388#discussion_r227604758
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java ---
@@ -1540,6 +1561,75 @@ public void run() {
}
}
+ private final class AddressQueueReaper extends ActiveMQScheduledComponent {
+
+ AddressQueueReaper(ScheduledExecutorService scheduledExecutorService,
+ Executor executor,
+ long checkPeriod,
+ TimeUnit timeUnit,
+ boolean onDemand) {
+ super(scheduledExecutorService, executor, checkPeriod, timeUnit, onDemand);
+ }
+
+ @Override
+ public void run() {
+ Map<SimpleString, Binding> nameMap = addressManager.getBindings();
+
+ List<Queue> queues = new ArrayList<>();
+
+ for (Binding binding : nameMap.values()) {
+ if (binding.getType() == BindingType.LOCAL_QUEUE) {
+ Queue queue = (Queue) binding.getBindable();
+
+ queues.add(queue);
+ }
+ }
+
+ for (Queue queue : queues) {
+ int consumerCount = queue.getConsumerCount();
+ long messageCount = queue.getMessageCount();
+ boolean autoCreated = queue.isAutoCreated();
+ long consumerRemovedTimestamp = queue.getConsumerRemovedTimestamp();
+
+ if (!queue.isInternalQueue() && autoCreated && messageCount == 0 && consumerCount == 0 && consumerRemovedTimestamp != -1) {
+ SimpleString queueName = queue.getName();
+ AddressSettings settings = addressSettingsRepository.getMatch(queue.getAddress().toString());
+ if (settings.isAutoDeleteQueues() && (System.currentTimeMillis() - consumerRemovedTimestamp >= settings.getAutoDeleteQueuesDelay())) {
+ if (ActiveMQServerLogger.LOGGER.isDebugEnabled()) {
+ ActiveMQServerLogger.LOGGER.info("deleting auto-created queue \"" + queueName + ".\" consumerCount = " + consumerCount + "; messageCount = " + messageCount + "; isAutoDeleteQueues = " + settings.isAutoDeleteQueues());
+ }
+
+ try {
+ server.destroyQueue(queueName, null, true, false);
--- End diff --
The chance of this existed (and still does) in the existing QueueManagerImpl logic where no delay is taken into account.
---