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 2020/06/04 23:20:03 UTC
[GitHub] [activemq-artemis] michaelandrepearce opened a new pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
michaelandrepearce opened a new pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169
Add feature
Add tests
Add docs
Add missing bits noticed in ring-size
----------------------------------------------------------------
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] michaelpearce-gain commented on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639554539
so thats taken care of if you update config via jmx console
https://github.com/apache/activemq-artemis/pull/3169/files#diff-1fae8ea746270e6f0b1f0c99a6148ab2
----------------------------------------------------------------
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] michaelpearce-gain edited a comment on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain edited a comment on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639554539
so thats taken care of if you update config via jmx console, e.g. you would call updateQueue passing in queueConfig json, and it has same behaviour as updating via config.xml
https://github.com/apache/activemq-artemis/pull/3169/files#diff-1fae8ea746270e6f0b1f0c99a6148ab2
----------------------------------------------------------------
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] michaelpearce-gain commented on a change in pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#discussion_r435653023
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/addressing/AddressingTest.java
##########
@@ -268,6 +268,62 @@ public void testPurgeOnNoConsumersFalse() throws Exception {
Wait.assertEquals(1, server.locateQueue(queueName)::getMessageCount);
}
+
+ @Test
+ public void testQueueEnabledDisabled() throws Exception {
Review comment:
@clebertsuconic see here this is the actual functionality test, where enabling / disabling the queue.
(other tests added are just around checking config works and updates etc should reload etc occur)
----------------------------------------------------------------
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 #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169
----------------------------------------------------------------
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] michaelandrepearce commented on a change in pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#discussion_r437274728
##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/wireformat/SessionQueueQueryResponseMessage_V3.java
##########
@@ -475,6 +501,11 @@ public boolean equals(Object obj) {
return false;
} else if (!ringSize.equals(other.ringSize))
return false;
+ if (enabled == enabled) {
Review comment:
good spot, https://github.com/apache/activemq-artemis/pull/3174 fix here.
----------------------------------------------------------------
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] michaelpearce-gain commented on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639212336
@clebertsuconic re
`I am trying to understand what disabling a queue would mean. There's already pause on a queue.`
pause only works when no consumers, in this case we need to stop messages entering the queue, even if consumers are attached.
----------------------------------------------------------------
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 #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#discussion_r436103152
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/QueueControlImpl.java
##########
@@ -618,6 +618,52 @@ public boolean isPurgeOnNoConsumers() {
}
}
+ @Override
+ public void disable() throws Exception {
+ if (AuditLogger.isEnabled()) {
+ AuditLogger.disable(queue);
+ }
+ checkStarted();
+
+ clearIO();
+ try {
+ server.getPostOffice().updateQueue(queue.getQueueConfiguration().setEnabled(false));
+ } finally {
+ blockOnIO();
+ }
+ }
+
+ @Override
+ public void enable() throws Exception {
+ if (AuditLogger.isEnabled()) {
+ AuditLogger.enable(queue);
+ }
+ checkStarted();
+
+ clearIO();
+ try {
+ server.getPostOffice().updateQueue(queue.getQueueConfiguration().setEnabled(true));
Review comment:
nice one... I will run tests and merge it
----------------------------------------------------------------
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 pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-640677792
I'm just confirming one non related failure on my last run. if everything ok I will merge this today.
----------------------------------------------------------------
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] michaelpearce-gain edited a comment on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain edited a comment on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639214686
@clebertsuconic its used by the queue to disable it thats what the feature adds, as such its not meta-data. I think @jbertram called it out already though but just incase needed elaborating.
----------------------------------------------------------------
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] michaelpearce-gain edited a comment on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain edited a comment on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639212336
@clebertsuconic re
`I am trying to understand what disabling a queue would mean. There's already pause on a queue.`
pause stops dispatch to consumers, but it means messages still route and enter the queue, causing it to fill up.
again there is also, purge on no consumer BUT it only works when no consumers, in this case we need to stop messages entering the queue, even if consumers are attached.
----------------------------------------------------------------
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] michaelpearce-gain commented on a change in pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#discussion_r435653023
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/addressing/AddressingTest.java
##########
@@ -268,6 +268,62 @@ public void testPurgeOnNoConsumersFalse() throws Exception {
Wait.assertEquals(1, server.locateQueue(queueName)::getMessageCount);
}
+
+ @Test
+ public void testQueueEnabledDisabled() throws Exception {
Review comment:
@clebertsuconic see here this is the actual functionality test, where enabling / disabling the queue.
other tests in the PR (which is probably what caused you to think there was no effect on the queue) were added are just around checking config works and updates etc should reload config etc occur.
----------------------------------------------------------------
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 #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639186313
@clebertsuconic, look at `QueueImpl.route()`. That's where the functionality is actually implemented.
----------------------------------------------------------------
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] michaelpearce-gain edited a comment on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain edited a comment on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639212336
@clebertsuconic re
`I am trying to understand what disabling a queue would mean. There's already pause on a queue.`
pause stops dispatch to consumers, but it means messages still route and enter the queue, causing it to fill up.
again there is also, purge on no consumer this does indeed stop messages entering the queue BUT it only works when no consumers
in this use case we need to stop messages entering the queue, even if consumers are attached. essentially, disabling the queue (thus enabled true/false aka enabled/disabled).
----------------------------------------------------------------
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 pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639174040
unless you tell me there's a function for disabling the queue on the broker... I would prefer to serve your feature with a generic metadata, that you set at your own desire. as I couldn't find a function for the broker itself.
----------------------------------------------------------------
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] michaelpearce-gain commented on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-640188906
@clebertsuconic fixed , the new case is only valid for new createQueue (not legacy , e.g the new unified createQueue(QueueConfig))
----------------------------------------------------------------
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] michaelpearce-gain commented on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639213407
@jbertram will add extra line to the docs to mention that.
----------------------------------------------------------------
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] michaelpearce-gain edited a comment on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain edited a comment on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-640188906
@clebertsuconic fixed , the new case setting enabled to false is only valid for new createQueue (not legacy , e.g the new unified createQueue(QueueConfig), as only exposed in the new one.
----------------------------------------------------------------
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] michaelpearce-gain commented on a change in pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#discussion_r435649023
##########
File path: artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/QueueAttributes.java
##########
@@ -103,6 +105,8 @@ public void set(String key, String value) {
setAutoDeleteMessageCount(Long.valueOf(value));
} else if (key.equals(RING_SIZE)) {
setRingSize(Long.valueOf(value));
+ } else if (key.equals(ENABLED)) {
+ setRingSize(Long.valueOf(value));
}
Review comment:
yes good catch
----------------------------------------------------------------
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] michaelpearce-gain edited a comment on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain edited a comment on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639212336
@clebertsuconic re
`I am trying to understand what disabling a queue would mean. There's already pause on a queue.`
pause stops dispatch to consumers, but it means messages still route and enter the queue, causing it to fill up.
again there is also, purge on no consumer BUT it only works when no consumers, in this case we need to stop messages entering the queue, even if consumers are attached.
----------------------------------------------------------------
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] michaelpearce-gain commented on a change in pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#discussion_r435996241
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/QueueControlImpl.java
##########
@@ -618,6 +618,52 @@ public boolean isPurgeOnNoConsumers() {
}
}
+ @Override
+ public void disable() throws Exception {
+ if (AuditLogger.isEnabled()) {
+ AuditLogger.disable(queue);
+ }
+ checkStarted();
+
+ clearIO();
+ try {
+ server.getPostOffice().updateQueue(queue.getQueueConfiguration().setEnabled(false));
+ } finally {
+ blockOnIO();
+ }
+ }
+
+ @Override
+ public void enable() throws Exception {
+ if (AuditLogger.isEnabled()) {
+ AuditLogger.enable(queue);
+ }
+ checkStarted();
+
+ clearIO();
+ try {
+ server.getPostOffice().updateQueue(queue.getQueueConfiguration().setEnabled(true));
Review comment:
now it updates the config, as such persists in the queue binding.
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/QueueControlImpl.java
##########
@@ -618,6 +618,52 @@ public boolean isPurgeOnNoConsumers() {
}
}
+ @Override
+ public void disable() throws Exception {
+ if (AuditLogger.isEnabled()) {
+ AuditLogger.disable(queue);
+ }
+ checkStarted();
+
+ clearIO();
+ try {
+ server.getPostOffice().updateQueue(queue.getQueueConfiguration().setEnabled(false));
+ } finally {
+ blockOnIO();
+ }
+ }
+
+ @Override
+ public void enable() throws Exception {
+ if (AuditLogger.isEnabled()) {
+ AuditLogger.enable(queue);
+ }
+ checkStarted();
+
+ clearIO();
+ try {
+ server.getPostOffice().updateQueue(queue.getQueueConfiguration().setEnabled(true));
Review comment:
@clebertsuconic now it updates the config, as such persists in the queue binding.
----------------------------------------------------------------
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 pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639234328
I missed the QueueImpl::route, I only saw the change the PostOfficeImpl...
it makes sense as a feature.
----------------------------------------------------------------
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 pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-640155730
@michaelandrepearce / @michaelpearce-gain a test you wrote is failing 100% of the time. Can you take a look?
org.apache.activemq.artemis.tests.integration.client.SessionTest::testCreateQueue
----------------------------------------------------------------
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 pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639551334
@michaelpearce-gain just one thing I'm wondering...
we have the hability to store pause / start on Queues.
wouldn't make sense to do the same? perhaps reuse the same record checking if there's more bytes at the end?
----------------------------------------------------------------
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 edited a comment on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
clebertsuconic edited a comment on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639551334
@michaelpearce-gain just one thing I'm wondering...
we have the hability to store pause / start on Queues on the journal.
wouldn't make sense to do the same? perhaps reuse the same record checking if there's more bytes at the end?
----------------------------------------------------------------
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 #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639187658
I think it would be worth making an explicit statement in the documentation that disabling all the queues on an address means that any message sent to that address will be silently dropped. I'm sure this point will be clear to savvy users, but others may not connect the dots as readily.
----------------------------------------------------------------
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] michaelpearce-gain commented on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639214686
@clebertsuconic its used by the queue to disable it thats what the feature adds, as such its not meta-data.
----------------------------------------------------------------
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] michaelpearce-gain commented on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639563296
----------------------------------------------------------------
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] michaelpearce-gain edited a comment on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain edited a comment on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639554539
----------------------------------------------------------------
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] michaelpearce-gain edited a comment on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain edited a comment on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639212336
@clebertsuconic re
`I am trying to understand what disabling a queue would mean. There's already pause on a queue.`
pause stops dispatch to consumers, but it means messages still route and enter the queue, causing it to fill up.
again there is also, purge on no consumer this does indeed stop messages entering the queue BUT it only works when no consumers (as well it has another consequence of purging the data in the queue)
in this use case we need to stop messages entering the queue, even if consumers are attached. essentially, disabling the queue (thus enabled true/false aka enabled/disabled).
----------------------------------------------------------------
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] michaelpearce-gain edited a comment on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain edited a comment on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639212336
@clebertsuconic re
`I am trying to understand what disabling a queue would mean. There's already pause on a queue.`
pause stops dispatch to consumers, but it means messages still route and enter the queue, causing it to fill up.
again there is also, purge on no consumer this does indeed stop messages entering the queue BUT it only works when no consumers (as well it has another consequence (unwanted) of purging the data in the queue automatically when first consumer re-attaches)
in this use case we need to simply stop messages entering the queue, even if consumers are attached. essentially, disabling the queue (thus enabled true/false aka enabled/disabled),
----------------------------------------------------------------
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] michaelpearce-gain edited a comment on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain edited a comment on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639563296
i see what you mean, i think holding state somewhere else (e.g. in two places) would be dangerou thoughs, we could though do the updae via post office, as such it will update the binding and then queue that way, as such it will go via same route as config change and state be held in the queue binding.
e.,g.
server.getPostOffice().updateQueue(queue.getQueueConfiguration().setEnabled(false));
----------------------------------------------------------------
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] michaelpearce-gain commented on a change in pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#discussion_r435653023
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/addressing/AddressingTest.java
##########
@@ -268,6 +268,62 @@ public void testPurgeOnNoConsumersFalse() throws Exception {
Wait.assertEquals(1, server.locateQueue(queueName)::getMessageCount);
}
+
+ @Test
+ public void testQueueEnabledDisabled() throws Exception {
Review comment:
@clebertsuconic see here this is the actual functionality test, where enabling / disabling the queue, and affects the behaviour (E.g. disables the actual queue receiving messages)
other tests in the PR (which is probably what caused you to think there was no effect on the queue) were added are just around checking config works and updates etc should reload config etc occur.
----------------------------------------------------------------
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] michaelpearce-gain edited a comment on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain edited a comment on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639554539
so thats taken care of if you update config via jmx console, e.g. you would call updateQueue passing in queueConfig json, and it has same behaviour as updating via config.xml, it updates the queue config just like any other config change on a queue, and is persited in the queue binding
https://github.com/apache/activemq-artemis/pull/3169/files#diff-1fae8ea746270e6f0b1f0c99a6148ab2
I wouldnt want queue settings persisted in two different journal place places, like other queue config, should be all in the persisted queue binding.
----------------------------------------------------------------
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] michaelpearce-gain commented on a change in pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#discussion_r435652202
##########
File path: docs/user-manual/en/address-model.md
##########
@@ -527,6 +527,34 @@ Open the file `<broker-instance>/etc/broker.xml` for editing.
</addresses>
```
+#### Disabled Queue
+
+If a user requires to statically configure a queue and disable routing to it,
+for example where a queue needs to be defined so a consumer can bind,
+but you want to disable message routing to it for the time being.
+
+Or you need to stop message flow to the queue to allow investigation keeping the consumer bound,
+but dont wish to have further messages routed to the queue to avoid message build up.
+
+When **enabled** is set to **true** the queue will have messages routed to it. (default)
+
+When **enabled** is set to **false** the queue will NOT have messages routed to it.
+
+Open the file `<broker-instance>/etc/broker.xml` for editing.
+
+```xml
+<addresses>
+ <address name="foo.bar">
+ <multicast>
+ <queue name="orders1" enabled="false"/>
+ </multicast>
+ </address>
+</addresses>
+```
+
+Warning: Disabling all the queues on an address means that any message sent to that address will be silently dropped.
Review comment:
@jbertram added doc in line to be explicit if someone disables all queues.
##########
File path: artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/QueueAttributes.java
##########
@@ -103,6 +105,8 @@ public void set(String key, String value) {
setAutoDeleteMessageCount(Long.valueOf(value));
} else if (key.equals(RING_SIZE)) {
setRingSize(Long.valueOf(value));
+ } else if (key.equals(ENABLED)) {
+ setRingSize(Long.valueOf(value));
}
Review comment:
fixed
----------------------------------------------------------------
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] michaelpearce-gain commented on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639500451
@clebertsuconic cool :) , anything else needed before merge? I fixed the bit justin spotted, and added the warning to docs regarding if someone disabled all.
----------------------------------------------------------------
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] michaelpearce-gain edited a comment on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain edited a comment on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639554539
so thats taken care of if you update config via jmx console, e.g. you would call updateQueue passing in queueConfig json, and it has same behaviour as updating via config.xml
https://github.com/apache/activemq-artemis/pull/3169/files#diff-1fae8ea746270e6f0b1f0c99a6148ab2
I wouldnt want queue settings persisted in two places, like other queue config, should be in the persisted queue binding.
----------------------------------------------------------------
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 pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-640155730
@michaelandrepearce / @michaelpearce-gain a test you wrote is failing 100% of the time. Can you take a look?
org.apache.activemq.artemis.tests.integration.client.SessionTest::testCreateQueue
----------------------------------------------------------------
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 a change in pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#discussion_r435624163
##########
File path: artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/QueueAttributes.java
##########
@@ -103,6 +105,8 @@ public void set(String key, String value) {
setAutoDeleteMessageCount(Long.valueOf(value));
} else if (key.equals(RING_SIZE)) {
setRingSize(Long.valueOf(value));
+ } else if (key.equals(ENABLED)) {
+ setRingSize(Long.valueOf(value));
}
Review comment:
Shouldn't this be `setEnabled(Boolean.valueOf(value));`?
----------------------------------------------------------------
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] michaelpearce-gain commented on pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639552937
@clebertsuconic i dont follow sorry. its stored in the queue config, as such if you update the queue config, as like any other queue config change it will be persisted.
----------------------------------------------------------------
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 pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639553449
@michaelandrepearce queue.pause / queue.resume.. there's an option on the JMX Console method that will store the value on the journal (not on the config)
I was just asking if wouldn't make sense to do the same.
----------------------------------------------------------------
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 pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639173760
The only outcome that I saw from setting enabled or disabled is the boolean changed itself.. being a metadata that serves no purpose to the broker itself.
wouldn't be best to implement a generic meta-data for this kind of purpose?
----------------------------------------------------------------
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 pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#issuecomment-639171937
I am trying to understand what disabling a queue would mean. There's already pause on a queue.
----------------------------------------------------------------
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] ehsavoie commented on a change in pull request #3169: ARTEMIS-2787 - Add ability to disable and enable a queue
Posted by GitBox <gi...@apache.org>.
ehsavoie commented on a change in pull request #3169:
URL: https://github.com/apache/activemq-artemis/pull/3169#discussion_r437261296
##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/wireformat/SessionQueueQueryResponseMessage_V3.java
##########
@@ -475,6 +501,11 @@ public boolean equals(Object obj) {
return false;
} else if (!ringSize.equals(other.ringSize))
return false;
+ if (enabled == enabled) {
Review comment:
This is wrong I think
----------------------------------------------------------------
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