You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2021/11/02 02:00:06 UTC
[GitHub] [activemq-artemis] clebertsuconic opened a new pull request #3833: ARTEMIS-3541 createQueue ignoring autoDeleteAddress flag
clebertsuconic opened a new pull request #3833:
URL: https://github.com/apache/activemq-artemis/pull/3833
Commit 481b73c8cabf9d7c88bca048b97bd587661b9c52 from ARTEMIS-3502
inadvertently broke this functionality. This commit restores the
original behavior.
autoDeleteAddress was renamed to forceAutoDeleteAddress which will ignore the address settings.
delete temporary queues will use forceAutoDeleteAddress=true.
--
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
[GitHub] [activemq-artemis] asfgit closed pull request #3833: ARTEMIS-3541 createQueue ignoring autoDeleteAddress flag
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3833:
URL: https://github.com/apache/activemq-artemis/pull/3833
--
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
[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3833: ARTEMIS-3541 createQueue ignoring autoDeleteAddress flag
Posted by GitBox <gi...@apache.org>.
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3833: ARTEMIS-3541 createQueue ignoring autoDeleteAddress flag
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3833:
URL: https://github.com/apache/activemq-artemis/pull/3833#issuecomment-957030514
@jbertram this is replacing your previous PR on this
--
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
[GitHub] [activemq-artemis] gemmellr commented on pull request #3833: ARTEMIS-3541 createQueue ignoring autoDeleteAddress flag
Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #3833:
URL: https://github.com/apache/activemq-artemis/pull/3833#issuecomment-958021224
PR title and commit message are wrong, as with the last ones. It was destroyQueue.
--
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
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3833: ARTEMIS-3541 createQueue ignoring autoDeleteAddress flag
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3833:
URL: https://github.com/apache/activemq-artemis/pull/3833#issuecomment-957030514
@jbertram this is replacing your previous PR on this
--
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
[GitHub] [activemq-artemis] jbertram commented on pull request #3833: ARTEMIS-3541 createQueue ignoring autoDeleteAddress flag
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3833:
URL: https://github.com/apache/activemq-artemis/pull/3833#issuecomment-957989132
@clebertsuconic, LGTM. Will merge ASAP.
--
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
[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3833: ARTEMIS-3541 createQueue ignoring autoDeleteAddress flag
Posted by GitBox <gi...@apache.org>.
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3833: ARTEMIS-3541 createQueue ignoring autoDeleteAddress flag
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3833:
URL: https://github.com/apache/activemq-artemis/pull/3833#discussion_r741393339
##########
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:
we could call it instantAutoDeleteAddress instead of force.
This is just doing the check on the address instantly as opposed to leave it to the reaper (in the new semantics now).
--
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
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3833: ARTEMIS-3541 createQueue ignoring autoDeleteAddress flag
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3833:
URL: https://github.com/apache/activemq-artemis/pull/3833#discussion_r741393339
##########
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:
@gemmellr we could call it instantAutoDeleteAddress instead of force.
This is just doing the check on the address instantly as opposed to leave it to the reaper (in the new semantics now).
--
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
[GitHub] [activemq-artemis] gemmellr commented on pull request #3833: ARTEMIS-3541 createQueue ignoring autoDeleteAddress flag
Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #3833:
URL: https://github.com/apache/activemq-artemis/pull/3833#issuecomment-958021224
PR title and commit message are wrong, as with the last ones. It was destroyQueue.
--
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
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3833: ARTEMIS-3541 createQueue ignoring autoDeleteAddress flag
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3833:
URL: https://github.com/apache/activemq-artemis/pull/3833#issuecomment-957030514
@jbertram this is replacing your previous PR on this
--
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
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3833: ARTEMIS-3541 createQueue ignoring autoDeleteAddress flag
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3833:
URL: https://github.com/apache/activemq-artemis/pull/3833#discussion_r741393339
##########
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:
we could call it instantAutoDeleteAddress instead of force.
This is just doing the check on the address instantly as opposed to leave it to the reaper (in the new semantics now).
##########
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:
@gemmellr we could call it instantAutoDeleteAddress instead of force.
This is just doing the check on the address instantly as opposed to leave it to the reaper (in the new semantics now).
##########
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:
@gemmellr lets move the discussion on the PR following up your notes please?:
https://github.com/apache/activemq-artemis/pull/3835
--
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
[GitHub] [activemq-artemis] jbertram commented on pull request #3833: ARTEMIS-3541 createQueue ignoring autoDeleteAddress flag
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3833:
URL: https://github.com/apache/activemq-artemis/pull/3833#issuecomment-957989132
@clebertsuconic, LGTM. Will merge ASAP.
--
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
[GitHub] [activemq-artemis] asfgit closed pull request #3833: ARTEMIS-3541 createQueue ignoring autoDeleteAddress flag
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3833:
URL: https://github.com/apache/activemq-artemis/pull/3833
--
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