[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3819: ARTEMIS-3541 createQueue ignoring autoDeleteAddress flag

[GitBox <gi...@apache.org>]

gemmellr commented on a change in pull request #3819:
URL: https://github.com/apache/activemq-artemis/pull/3819#discussion_r737653829



##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java
##########
@@ -2397,15 +2397,13 @@ public void destroyQueue(final SimpleString queueName,
             callBrokerQueuePlugins(plugin -> plugin.afterDestroyQueue(queue, address, session, checkConsumerCount, removeConsumers, autoDeleteAddress));
          }
 
-         if (queue.isTemporary()) {
-            AddressInfo addressInfo = getAddressInfo(address);
+         AddressInfo addressInfo = getAddressInfo(address);
 
-            if (autoDeleteAddress && postOffice != null && addressInfo != null && addressInfo.isAutoCreated() && !isAddressBound(address.toString()) && addressSettingsRepository.getMatch(address.toString()).getAutoDeleteAddressesDelay() == 0) {
-               try {
-                  removeAddressInfo(address, session);
-               } catch (ActiveMQDeleteAddressException e) {
-                  // Could be thrown if the address has bindings or is not deletable.
-               }
+         if (autoDeleteAddress && postOffice != null && addressInfo != null && addressInfo.isAutoCreated() && !isAddressBound(address.toString()) && addressSettingsRepository.getMatch(address.toString()).getAutoDeleteAddressesDelay() == 0) {

Review comment:
       > you should instead make sure the AddressReaper would remove such addresses.. I would revert this fix here and would add a check for auto-delete address on the reaper.
   
   Do you mean adding some sort of additional flag that this method would set (if autoDeleteAddress was passed true by management) to indicate the reaper should later delete the address even when it otherwise wouldnt? The reaper is already checking whether it should auto-delete an address, but decides not to for this case since auto-delete-addresses is false in the broker config.
   
   Its not clear to me that it shouldnt just be deleted here, so long as it is a request from management (as earlier, the impl would need adjusted to account for what it is being used for). The queue already has been at this point, and the alternative if there was no autoDeleteAddress boolean on destroyQueue would presumably be to go and perform an explicit deletion on the address afterwards as well using management, which I assume also acts immediately.




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org