You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by "AntonRoskvist (via GitHub)" <gi...@apache.org> on 2024/02/29 14:15:27 UTC
[PR] ARTEMIS-4664 - autoCreatedResource can get removed while receiving ba… [activemq-artemis]
AntonRoskvist opened a new pull request, #4836:
URL: https://github.com/apache/activemq-artemis/pull/4836
…tch of messages
This is probably not all that likely to ever happen to anyone. I stumbled on this issue while working on something else but I figured it would be better to handle it.
From what I can tell the auto created resource would have to have been empty for one pass of the "AddressQueueReaper" to be marked as "swept", then a large batch of messages would get sent to the resource just as the next pass of "AddressQueueReaper" begins and finishes before the message batch finishes.
--
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
Re: [PR] ARTEMIS-4664 - autoCreatedResource can get removed while receiving ba… [activemq-artemis]
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4836:
URL: https://github.com/apache/activemq-artemis/pull/4836#discussion_r1515159666
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
##########
@@ -3919,7 +3919,10 @@ private void createResources(boolean isAutoCreate, SimpleString destinationAddre
SimpleString destinationQueueName = prefix.concat(getAddress()).concat(suffix);
SimpleString filter = new SimpleString(String.format("%s = '%s'", Message.HDR_ORIGINAL_ADDRESS, getAddress()));
try {
- server.createQueue(new QueueConfiguration(destinationQueueName).setAddress(destinationAddress).setFilterString(filter).setAutoCreated(true).setAutoCreateAddress(true), true);
+ Queue destinationQueue = server.createQueue(new QueueConfiguration(destinationQueueName).setAddress(destinationAddress).setFilterString(filter).setAutoCreated(true).setAutoCreateAddress(true), true);
+ if (destinationQueue != null && destinationQueue.isSwept()) {
+ destinationQueue.setSwept(false);
Review Comment:
The sweep is only used for the reaper. So it wouldn't hurt to set it to false in there.
I could say set it if it was auto created. But it shouldn't matter.
--
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
Re: [PR] ARTEMIS-4664 - autoCreatedResource can get removed while receiving ba… [activemq-artemis]
Posted by "AntonRoskvist (via GitHub)" <gi...@apache.org>.
AntonRoskvist commented on code in PR #4836:
URL: https://github.com/apache/activemq-artemis/pull/4836#discussion_r1515137779
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
##########
@@ -3919,7 +3919,10 @@ private void createResources(boolean isAutoCreate, SimpleString destinationAddre
SimpleString destinationQueueName = prefix.concat(getAddress()).concat(suffix);
SimpleString filter = new SimpleString(String.format("%s = '%s'", Message.HDR_ORIGINAL_ADDRESS, getAddress()));
try {
- server.createQueue(new QueueConfiguration(destinationQueueName).setAddress(destinationAddress).setFilterString(filter).setAutoCreated(true).setAutoCreateAddress(true), true);
+ Queue destinationQueue = server.createQueue(new QueueConfiguration(destinationQueueName).setAddress(destinationAddress).setFilterString(filter).setAutoCreated(true).setAutoCreateAddress(true), true);
+ if (destinationQueue != null && destinationQueue.isSwept()) {
+ destinationQueue.setSwept(false);
Review Comment:
Sure, that's no problem. The intent being to also catch some similar circumstances outside of the auto created resources, right?
--
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
Re: [PR] ARTEMIS-4664 - autoCreatedResource can get removed while receiving ba… [activemq-artemis]
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4836:
URL: https://github.com/apache/activemq-artemis/pull/4836#discussion_r1515388941
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
##########
@@ -3919,7 +3919,10 @@ private void createResources(boolean isAutoCreate, SimpleString destinationAddre
SimpleString destinationQueueName = prefix.concat(getAddress()).concat(suffix);
SimpleString filter = new SimpleString(String.format("%s = '%s'", Message.HDR_ORIGINAL_ADDRESS, getAddress()));
try {
- server.createQueue(new QueueConfiguration(destinationQueueName).setAddress(destinationAddress).setFilterString(filter).setAutoCreated(true).setAutoCreateAddress(true), true);
+ Queue destinationQueue = server.createQueue(new QueueConfiguration(destinationQueueName).setAddress(destinationAddress).setFilterString(filter).setAutoCreated(true).setAutoCreateAddress(true), true);
+ if (destinationQueue != null && destinationQueue.isSwept()) {
+ destinationQueue.setSwept(false);
Review Comment:
I run my complete CI with your changes (before my requested change) and they were all good.
I will merge as soon as you amend the change.
--
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
Re: [PR] ARTEMIS-4664 - autoCreatedResource can get removed while receiving ba… [activemq-artemis]
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic merged PR #4836:
URL: https://github.com/apache/activemq-artemis/pull/4836
--
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
Re: [PR] ARTEMIS-4664 - autoCreatedResource can get removed while receiving ba… [activemq-artemis]
Posted by "AntonRoskvist (via GitHub)" <gi...@apache.org>.
AntonRoskvist commented on code in PR #4836:
URL: https://github.com/apache/activemq-artemis/pull/4836#discussion_r1515958109
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
##########
@@ -3919,7 +3919,10 @@ private void createResources(boolean isAutoCreate, SimpleString destinationAddre
SimpleString destinationQueueName = prefix.concat(getAddress()).concat(suffix);
SimpleString filter = new SimpleString(String.format("%s = '%s'", Message.HDR_ORIGINAL_ADDRESS, getAddress()));
try {
- server.createQueue(new QueueConfiguration(destinationQueueName).setAddress(destinationAddress).setFilterString(filter).setAutoCreated(true).setAutoCreateAddress(true), true);
+ Queue destinationQueue = server.createQueue(new QueueConfiguration(destinationQueueName).setAddress(destinationAddress).setFilterString(filter).setAutoCreated(true).setAutoCreateAddress(true), true);
+ if (destinationQueue != null && destinationQueue.isSwept()) {
+ destinationQueue.setSwept(false);
Review Comment:
Should be done now, hope it looks good... only thing I'm considering is if the `setSwept` should be done before the `ignoreIfExists`-check instead of after? Decided against it for now but I can change again if that makes sense.
--
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
Re: [PR] ARTEMIS-4664 - autoCreatedResource can get removed while receiving ba… [activemq-artemis]
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4836:
URL: https://github.com/apache/activemq-artemis/pull/4836#discussion_r1515136657
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
##########
@@ -3919,7 +3919,10 @@ private void createResources(boolean isAutoCreate, SimpleString destinationAddre
SimpleString destinationQueueName = prefix.concat(getAddress()).concat(suffix);
SimpleString filter = new SimpleString(String.format("%s = '%s'", Message.HDR_ORIGINAL_ADDRESS, getAddress()));
try {
- server.createQueue(new QueueConfiguration(destinationQueueName).setAddress(destinationAddress).setFilterString(filter).setAutoCreated(true).setAutoCreateAddress(true), true);
+ Queue destinationQueue = server.createQueue(new QueueConfiguration(destinationQueueName).setAddress(destinationAddress).setFilterString(filter).setAutoCreated(true).setAutoCreateAddress(true), true);
+ if (destinationQueue != null && destinationQueue.isSwept()) {
+ destinationQueue.setSwept(false);
Review Comment:
Meaning.. if anyone called createQueue, and it already existed.. the swept should be set to false I think. that means the fix would apply to any other case where createQueue was called.
--
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
Re: [PR] ARTEMIS-4664 - autoCreatedResource can get removed while receiving ba… [activemq-artemis]
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4836:
URL: https://github.com/apache/activemq-artemis/pull/4836#discussion_r1515116973
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
##########
@@ -3919,7 +3919,10 @@ private void createResources(boolean isAutoCreate, SimpleString destinationAddre
SimpleString destinationQueueName = prefix.concat(getAddress()).concat(suffix);
SimpleString filter = new SimpleString(String.format("%s = '%s'", Message.HDR_ORIGINAL_ADDRESS, getAddress()));
try {
- server.createQueue(new QueueConfiguration(destinationQueueName).setAddress(destinationAddress).setFilterString(filter).setAutoCreated(true).setAutoCreateAddress(true), true);
+ Queue destinationQueue = server.createQueue(new QueueConfiguration(destinationQueueName).setAddress(destinationAddress).setFilterString(filter).setAutoCreated(true).setAutoCreateAddress(true), true);
+ if (destinationQueue != null && destinationQueue.isSwept()) {
+ destinationQueue.setSwept(false);
Review Comment:
shouldn't this be done here instead?
https://github.com/apache/activemq-artemis/blob/5ce70f9e37ba1bb7c56372c2e79f9e9d8c9d1fa5/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java#L4093
--
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
Re: [PR] ARTEMIS-4664 - autoCreatedResource can get removed while receiving ba… [activemq-artemis]
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4836:
URL: https://github.com/apache/activemq-artemis/pull/4836#discussion_r1515388648
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
##########
@@ -3919,7 +3919,10 @@ private void createResources(boolean isAutoCreate, SimpleString destinationAddre
SimpleString destinationQueueName = prefix.concat(getAddress()).concat(suffix);
SimpleString filter = new SimpleString(String.format("%s = '%s'", Message.HDR_ORIGINAL_ADDRESS, getAddress()));
try {
- server.createQueue(new QueueConfiguration(destinationQueueName).setAddress(destinationAddress).setFilterString(filter).setAutoCreated(true).setAutoCreateAddress(true), true);
+ Queue destinationQueue = server.createQueue(new QueueConfiguration(destinationQueueName).setAddress(destinationAddress).setFilterString(filter).setAutoCreated(true).setAutoCreateAddress(true), true);
+ if (destinationQueue != null && destinationQueue.isSwept()) {
+ destinationQueue.setSwept(false);
Review Comment:
you don't need to check if isSwept... just setSwept(false) always on here:
https://github.com/apache/activemq-artemis/blob/5ce70f9e37ba1bb7c56372c2e79f9e9d8c9d1fa5/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java#L4093
--
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