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/02/02 21:50:15 UTC
[GitHub] [activemq-artemis] brusdev opened a new pull request #3428: ARTEMIS-3075 Skip temporary queues scale down
brusdev opened a new pull request #3428:
URL: https://github.com/apache/activemq-artemis/pull/3428
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3428: ARTEMIS-3075 Skip temporary queues scale down
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3428:
URL: https://github.com/apache/activemq-artemis/pull/3428#discussion_r568963622
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ScaleDownHandler.java
##########
@@ -124,9 +124,11 @@ public long scaleDownMessages(ClientSessionFactory sessionFactory,
for (Binding binding : bindings.getBindings()) {
if (binding instanceof LocalQueueBinding) {
Queue queue = ((LocalQueueBinding) binding).getQueue();
- // as part of scale down we will cancel any scheduled message and pass it to theWhile we scan for the queues we will also cancel any scheduled messages and deliver them right away
- queue.deliverScheduledMessages();
- queues.add(queue);
+ if (!queue.isTemporary()) {
+ // as part of scale down we will cancel any scheduled message and pass it to theWhile we scan for the queues we will also cancel any scheduled messages and deliver them right away
+ queue.deliverScheduledMessages();
Review comment:
this should handle only scheduled messages...
what about scaleDownRegularMessages?
or this issue is only only temporary messages?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram edited a comment on pull request #3428: ARTEMIS-3075 Skip temporary queues scale down
Posted by GitBox <gi...@apache.org>.
jbertram edited a comment on pull request #3428:
URL: https://github.com/apache/activemq-artemis/pull/3428#issuecomment-772593216
One of the benefits of scale-down as opposed to normal fail-over is that non-persistent messages are preserved. In normal fail-over only persistent messages are preserved.
It is, of course, normal for temporary queues to disappear when a client disconnects, but it also might be nice for scale-down to preserve these temporary queues so clients can reconnect and continue their work since the client itself was not responsible for the disconnect. For normal clients using temporary queues who get disconnect because some kind of spurious network issue they are able to reconnect back to their temporary queue unless the connection ttl has elapsed and the broker has cleaned up the queue. This is discussed in [the documentation](http://activemq.apache.org/components/artemis/documentation/latest/client-reconnection.html).
We could always deal with queues on the notifications address in a special way. For example, checking their name against the bridge's naming convention and removing queues we know will be orphaned.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #3428: ARTEMIS-3075 Skip temporary queues scale down
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3428:
URL: https://github.com/apache/activemq-artemis/pull/3428#issuecomment-772610322
@brusdev, that's definitely a problem. If we decide to support temporary queues we would need to fix that.
I also just realized that any temporary queue created during scale-down would not be directly tied to a client connection which means we'd have to clean them up some other way. We already have `org.apache.activemq.artemis.core.postoffice.impl.PostOfficeImpl.AddressQueueReaper` which could probably do the work, but it might be kind of fragile.
At this point it's probably best to just skip the temp queues as you've done. Keeping them would be nice, but it would almost certainly be a mess to implement.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3428: ARTEMIS-3075 Skip temporary queues scale down
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3428:
URL: https://github.com/apache/activemq-artemis/pull/3428#discussion_r569492163
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ScaleDownHandler.java
##########
@@ -124,9 +124,11 @@ public long scaleDownMessages(ClientSessionFactory sessionFactory,
for (Binding binding : bindings.getBindings()) {
if (binding instanceof LocalQueueBinding) {
Queue queue = ((LocalQueueBinding) binding).getQueue();
- // as part of scale down we will cancel any scheduled message and pass it to theWhile we scan for the queues we will also cancel any scheduled messages and deliver them right away
- queue.deliverScheduledMessages();
- queues.add(queue);
+ if (!queue.isTemporary()) {
+ // as part of scale down we will cancel any scheduled message and pass it to theWhile we scan for the queues we will also cancel any scheduled messages and deliver them right away
+ queue.deliverScheduledMessages();
Review comment:
Ohhh.. never mind... queues.add
I thought there was another loop looking for the search and I missed it.. ignore me.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3428: ARTEMIS-3075 Skip temporary queues scale down
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3428:
URL: https://github.com/apache/activemq-artemis/pull/3428#discussion_r568963622
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ScaleDownHandler.java
##########
@@ -124,9 +124,11 @@ public long scaleDownMessages(ClientSessionFactory sessionFactory,
for (Binding binding : bindings.getBindings()) {
if (binding instanceof LocalQueueBinding) {
Queue queue = ((LocalQueueBinding) binding).getQueue();
- // as part of scale down we will cancel any scheduled message and pass it to theWhile we scan for the queues we will also cancel any scheduled messages and deliver them right away
- queue.deliverScheduledMessages();
- queues.add(queue);
+ if (!queue.isTemporary()) {
+ // as part of scale down we will cancel any scheduled message and pass it to theWhile we scan for the queues we will also cancel any scheduled messages and deliver them right away
+ queue.deliverScheduledMessages();
Review comment:
this should handle only scheduled messages...
what about scaleDownRegularMessages?
or this issue is only only temporary messages?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] asfgit closed pull request #3428: ARTEMIS-3075 Skip temporary queues scale down
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3428:
URL: https://github.com/apache/activemq-artemis/pull/3428
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #3428: ARTEMIS-3075 Skip temporary queues scale down
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3428:
URL: https://github.com/apache/activemq-artemis/pull/3428#issuecomment-772593216
One of the benefits of scale-down as opposed to normal fail-over is that non-persistent messages are preserved. In normal fail-over only persistent messages are preserved.
It is, of course, normal for temporary queues to disappear when a client disconnects, but it also might be nice for scale-down to preserve these temporary queues so clients can reconnect and continue their work since the client itself was not responsible for the disconnect. For normal clients using temporary queues who get disconnect because some kind of spurious network issue they are able to reconnect back to their temporary queue unless the connection ttl has elapsed and the broker has cleaned up the queue. This is discussed in [the documentation](http://activemq.apache.org/components/artemis/documentation/latest/client-reconnection.html).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] brusdev commented on pull request #3428: ARTEMIS-3075 Skip temporary queues scale down
Posted by GitBox <gi...@apache.org>.
brusdev commented on pull request #3428:
URL: https://github.com/apache/activemq-artemis/pull/3428#issuecomment-772604588
@jbertram I see your point my only concern is that the scale-down transforms temporary queues in not temporary queues. If no client will consume those queues after the scale-down they will never be cleaned up.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org