gemmellr commented on a change in pull request #3833:
URL: https://github.com/apache/activemq-artemis/pull/3833#discussion_r741347945
##########
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueManagerImpl.java
##########
@@ -72,7 +72,7 @@ public static void performAutoDeleteQueue(ActiveMQServer
server, Queue queue) {
ActiveMQServerLogger.LOGGER.autoRemoveQueue("" + queue.getName(),
queue.getID(), "" + queue.getAddress());
try {
- server.destroyQueue(queueName, null, true, false,
settings.isAutoDeleteAddresses(), true);
+ server.destroyQueue(queueName, null, true, false, false, true);
Review comment:
This almost certainly rendered the settings unused, so they should have
been removed as well.
##########
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java
##########
@@ -672,13 +672,13 @@ void destroyQueue(SimpleString queueName,
SecurityAuth session,
boolean checkConsumerCount,
boolean removeConsumers,
- boolean autoDeleteAddress) throws Exception;
+ boolean forceAutoDeleteAddress) throws Exception;
void destroyQueue(SimpleString queueName,
SecurityAuth session,
boolean checkConsumerCount,
boolean removeConsumers,
- boolean autoDeleteAddress,
+ boolean forceAutoDeleteAddress,
Review comment:
Could use some doc here too to explain what it does.
##########
File path:
artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/ActiveMQServerControl.java
##########
@@ -1090,7 +1090,7 @@ void destroyQueue(@Parameter(name = "name", desc = "Name
of the queue to destroy
@Operation(desc = "Destroy a queue", impact = MBeanOperationInfo.ACTION)
void destroyQueue(@Parameter(name = "name", desc = "Name of the queue to
destroy") String name,
@Parameter(name = "removeConsumers", desc = "Remove
consumers of this queue") boolean removeConsumers,
- @Parameter(name = "autoDeleteAddress", desc =
"Automatically delete the address if this was the last queue") boolean
autoDeleteAddress) throws Exception;
+ @Parameter(name = "forceAutoDeleteAddress", desc =
"Automatically delete the address if this was the last queue") boolean
autoDeleteAddress) throws Exception;
Review comment:
This name could be fairly misleading with that doc along with it, which
isnt precisely what it does. I would consider it reasonable to think think a
'force' auto deletion described that way would always delete and do it there
and then, which it doesnt necessarily. It only deletes due to this if the
address was itself auto-created, and only if it has an exactly-0
autoDeleteDelay.
This would again give super-surprising behaviour if auto-delete-delay is
also disabled in the broker config generally, as it was leading to finding this
issue, since the address would then never be removed (by the reaper), even
despite this requested 'forcing'.
--
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]
