[GitHub] activemq-artemis issue #2373: ARTEMIS-2125 Tabs preference changes to displa...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2373 Lgtm ---
Re: [discussion] About blocking producers
Because our brokers are fully secured, if we need to stop incoming messages we simply remove the produce authorisation for users using security settings. This seems to work. Is this maybe an option for you? Sent from my Samsung Galaxy smartphone. Original message From: Howard Gao Date: 17/10/2018 04:02 (GMT+01:00) To: dev@activemq.apache.org Subject: Re: [discussion] About blocking producers Thanks Gary, you are quite right. This seems not a simple problem as I think is. The pause/resume is even harder to implement. And also the credit thing, some client support it (but may or may not use it), some are totally without it (like mqtt and stomp). Howard On Tue, Oct 16, 2018 at 6:57 PM Gary Tully wrote: > There may be two different use cases here; > 1) the pause/resume use case, where it makes some sense to leave producers > active stop reading/deny credit/buffer etc; the corollary to the pause > consumers > 2) the take down for maintenance case, where it would be nice to drain the > broker. > In the past, users have tackled the "maintenance" use case with > separate acceptors, one for producers and one for consumers. with the > ability to disable an acceptor via a management operation. That is not > ideal b/c it requires cooperation with the applications to use the correct > acceptors etc. > However it is a coarse grained option that is a step to shutdown, a one > way step that won't be undone via resume. That can more easily be > implemented with a management op that kills off producers and blocks all > send operations with some deny interceptor (along the lines of the > authorisation interceptors). something like shutdownSenders > > If the use case is more like shutdownSenders keeping it coarse (on or off > for the entire server) will be best. > > For the pause/resume case it gets tricky; > - for example if the same connection is used for producing and consuming, > any exception can be fatal for the connection, causing retries etc and > looping. > - blocking will only work for sync send operations and async operations > may accumulate. It may require protocol specific smarts. > - same for of credit, only where flow control is respected by the client. > > I guess my point is that pause/resume may not be the way to go if the use > case is actually shut down for maintenance. > > > On Tue, 16 Oct 2018 at 04:19 Howard Gao wrote: > > > Hi guys, > > > > I'm working on this story and I'd like to hear your thoughts. > > > > The Jira: https://issues.apache.org/jira/browse/ARTEMIS-2097 > > > > This is to provide a functionality of the broker to block on producers so > > that user can consume all the existing messages (for maintaining their > > servers for example), regardless of how the address setting's address > full > > policy is configured. The unblock > > functionality should also be provided to complete this feature. > > > > Here is my current implementation: > > https://github.com/apache/activemq-artemis/pull/2371 > > > > The idea is to keep a list of blocked/unblock addresses set by the user. > > Any incoming messages will be checked against the list and if its address > > is blocked, it will be rejected and an exception will be thrown to the > > clients. It works for all types of clients (core, openwire, jms, mqtt > > etc). > > > > Any comments are welcome. > > > > Thanks > > Howard > > >
[GitHub] activemq-artemis pull request #2369: ARTEMIS-2123 Paging not stopped if ther...
Github user wy96f commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2369#discussion_r225769985
--- Diff:
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageCursorProviderImpl.java
---
@@ -599,6 +600,18 @@ private long checkMinPage(Collection
cursorList) {
}
+ private void deliverIfNecessary(Collection
cursorList, long minPage) {
+ for (PageSubscription cursor : cursorList) {
+ long firstPage = cursor.getFirstPage();
+ if (firstPage == minPage) {
+if (cursor.getQueue().getMessageCount() == 0 &&
cursorList.size() > 1) {
+ cursor.getQueue().deliverAsync();
+ break;
--- End diff --
@wy96f @clebertsuconic Maybe more than one. I changed the code and added
another test. Please review the code:)
---
Re: [discussion] About blocking producers
Thanks Gary, you are quite right. This seems not a simple problem as I think is. The pause/resume is even harder to implement. And also the credit thing, some client support it (but may or may not use it), some are totally without it (like mqtt and stomp). Howard On Tue, Oct 16, 2018 at 6:57 PM Gary Tully wrote: > There may be two different use cases here; > 1) the pause/resume use case, where it makes some sense to leave producers > active stop reading/deny credit/buffer etc; the corollary to the pause > consumers > 2) the take down for maintenance case, where it would be nice to drain the > broker. > In the past, users have tackled the "maintenance" use case with > separate acceptors, one for producers and one for consumers. with the > ability to disable an acceptor via a management operation. That is not > ideal b/c it requires cooperation with the applications to use the correct > acceptors etc. >However it is a coarse grained option that is a step to shutdown, a one > way step that won't be undone via resume. That can more easily be > implemented with a management op that kills off producers and blocks all > send operations with some deny interceptor (along the lines of the > authorisation interceptors). something like shutdownSenders > > If the use case is more like shutdownSenders keeping it coarse (on or off > for the entire server) will be best. > > For the pause/resume case it gets tricky; > - for example if the same connection is used for producing and consuming, > any exception can be fatal for the connection, causing retries etc and > looping. > - blocking will only work for sync send operations and async operations > may accumulate. It may require protocol specific smarts. > - same for of credit, only where flow control is respected by the client. > > I guess my point is that pause/resume may not be the way to go if the use > case is actually shut down for maintenance. > > > On Tue, 16 Oct 2018 at 04:19 Howard Gao wrote: > > > Hi guys, > > > > I'm working on this story and I'd like to hear your thoughts. > > > > The Jira: https://issues.apache.org/jira/browse/ARTEMIS-2097 > > > > This is to provide a functionality of the broker to block on producers so > > that user can consume all the existing messages (for maintaining their > > servers for example), regardless of how the address setting's address > full > > policy is configured. The unblock > > functionality should also be provided to complete this feature. > > > > Here is my current implementation: > > https://github.com/apache/activemq-artemis/pull/2371 > > > > The idea is to keep a list of blocked/unblock addresses set by the user. > > Any incoming messages will be checked against the list and if its address > > is blocked, it will be rejected and an exception will be thrown to the > > clients. It works for all types of clients (core, openwire, jms, mqtt > > etc). > > > > Any comments are welcome. > > > > Thanks > > Howard > > >
[GitHub] activemq-artemis issue #2373: ARTEMIS-2125 Tabs preference changes to displa...
Github user tadayosi commented on the issue: https://github.com/apache/activemq-artemis/pull/2373 Thanks @franz1981. This looks great! ---
[GitHub] activemq-artemis pull request #2370: ARTEMIS-2125 Queue preference changes t...
Github user tadayosi closed the pull request at: https://github.com/apache/activemq-artemis/pull/2370 ---
[GitHub] activemq-artemis issue #2362: ARTEMIS-2117 Add custom LVQ Key and Non Destru...
Github user clebertsuconic commented on the issue: https://github.com/apache/activemq-artemis/pull/2362 I had to take kids to a piano lesson tonight. I donât have a computer now. I will have to connect later tonight. I will do some debug before I post as well. (Will run it again ) ---
[GitHub] activemq-artemis issue #2362: ARTEMIS-2117 Add custom LVQ Key and Non Destru...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2362 @clebertsuconic if you could share, im a bit surprised its this, as the new features should only impact if enabled, so wan't expecting any issues in old cases. Obviously like always happy to look at, as something could have been overlooked ---
[GitHub] activemq-artemis issue #2362: ARTEMIS-2117 Add custom LVQ Key and Non Destru...
Github user clebertsuconic commented on the issue: https://github.com/apache/activemq-artemis/pull/2362 the testsuite is stalling... I will have to figure out where and put some context here. ---
[GitHub] activemq-artemis pull request #2375: NO-JIRA small fix to avoid re-distribut...
GitHub user michaelandrepearce opened a pull request: https://github.com/apache/activemq-artemis/pull/2375 NO-JIRA small fix to avoid re-distributor being assigned a group You can merge this pull request into a Git repository by running: $ git pull https://github.com/michaelandrepearce/activemq-artemis redistributor Alternatively you can review and apply these changes as the patch at: https://github.com/apache/activemq-artemis/pull/2375.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #2375 commit 1e069c68b20035efa83d49a1d82a5fc2ccd040ca Author: Michael André Pearce Date: 2018-10-16T21:55:24Z NO-JIRA small fix to avoid re-distributor being assigned a group ---
[GitHub] activemq-artemis pull request #2362: ARTEMIS-2117 Add custom LVQ Key and Non...
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2362#discussion_r225695887
--- Diff:
artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/Message.java
---
@@ -186,10 +186,12 @@ default Message setRoutingType(RoutingType
routingType) {
return this;
}
+ @Deprecated
--- End diff --
removed
---
[GitHub] activemq pull request #310: [AMQ-7076] Fix and update features to fully supp...
Github user asfgit closed the pull request at: https://github.com/apache/activemq/pull/310 ---
[GitHub] activemq-artemis issue #2362: ARTEMIS-2117 Add custom LVQ Key and Non Destru...
Github user clebertsuconic commented on the issue: https://github.com/apache/activemq-artemis/pull/2362 I'm running a whole testsuite before we can merge this.. will have results in 3 hours. ---
[GitHub] activemq-artemis pull request #2362: ARTEMIS-2117 Add custom LVQ Key and Non...
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2362#discussion_r225585632
--- Diff:
artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/Message.java
---
@@ -186,10 +186,12 @@ default Message setRoutingType(RoutingType
routingType) {
return this;
}
+ @Deprecated
--- End diff --
No need ill remove
---
[GitHub] activemq-artemis pull request #2374: ARTEMIS-2127 Add auth details to consum...
Github user jbertram commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/2374#discussion_r225561623 --- Diff: docs/user-manual/en/management.md --- @@ -735,8 +735,8 @@ un-formatted result of a call to `java.lang.System.currentTimeMillis()`. - `CONSUMER_CREATED` (2) `_AMQ_Address`, `_AMQ_ClusterName`, `_AMQ_RoutingName`, `_AMQ_Distance`, - `_AMQ_ConsumerCount`, `_AMQ_User`, `_AMQ_RemoteAddress`, - `_AMQ_SessionName`, `_AMQ_FilterString` + `_AMQ_ConsumerCount`, `_AMQ_User`, `_AMQ_ValicatedUser`, `_AMQ_RemoteAddress`, --- End diff -- Nice catch! I just pushed a fix. ---
[GitHub] activemq-artemis pull request #2374: ARTEMIS-2127 Add auth details to consum...
Github user sebthom commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/2374#discussion_r225560766 --- Diff: docs/user-manual/en/management.md --- @@ -735,8 +735,8 @@ un-formatted result of a call to `java.lang.System.currentTimeMillis()`. - `CONSUMER_CREATED` (2) `_AMQ_Address`, `_AMQ_ClusterName`, `_AMQ_RoutingName`, `_AMQ_Distance`, - `_AMQ_ConsumerCount`, `_AMQ_User`, `_AMQ_RemoteAddress`, - `_AMQ_SessionName`, `_AMQ_FilterString` + `_AMQ_ConsumerCount`, `_AMQ_User`, `_AMQ_ValicatedUser`, `_AMQ_RemoteAddress`, --- End diff -- I think there is a typo: _AMQ_Vali**c**atedUser ->_AMQ_Vali**d**atedUser ---
[GitHub] activemq-artemis issue #2362: ARTEMIS-2117 Add custom LVQ Key and Non Destru...
Github user clebertsuconic commented on the issue: https://github.com/apache/activemq-artemis/pull/2362 My colleagues educated me on the non-descructive. Only question I have now is the deprecated method. ---
[GitHub] activemq-artemis pull request #2374: ARTEMIS-2127 Add auth details to consum...
GitHub user jbertram opened a pull request: https://github.com/apache/activemq-artemis/pull/2374 ARTEMIS-2127 Add auth details to consumer created notification You can merge this pull request into a Git repository by running: $ git pull https://github.com/jbertram/activemq-artemis ARTEMIS-2127 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/activemq-artemis/pull/2374.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #2374 commit 9d85168bcb18e5885ca8a7d394b7469df3f89c96 Author: Justin Bertram Date: 2018-10-16T13:01:45Z ARTEMIS-2127 Add auth details to consumer created notification ---
[GitHub] activemq-artemis pull request #2362: ARTEMIS-2117 Add custom LVQ Key and Non...
Github user clebertsuconic commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2362#discussion_r225527717
--- Diff:
artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/Message.java
---
@@ -186,10 +186,12 @@ default Message setRoutingType(RoutingType
routingType) {
return this;
}
+ @Deprecated
--- End diff --
Why deprecate this?
AMQP has some properties within the Spec that could be used. So this could
be Protocol Specific. Hence why we had this.
---
[GitHub] activemq-artemis issue #2362: ARTEMIS-2117 Add custom LVQ Key and Non Destru...
Github user clebertsuconic commented on the issue: https://github.com/apache/activemq-artemis/pull/2362 What is Non Destructive? I had a hard time understanding the use case. ---
[GitHub] activemq-artemis pull request #2345: ARTEMIS-2108 Potential StackOverflowErr...
Github user jbertram commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2345#discussion_r225517433
--- Diff:
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/distribution/RemoteBindingWithoutLoadBalancingTest.java
---
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.cluster.distribution;
+
+import
org.apache.activemq.artemis.core.server.cluster.impl.MessageLoadBalancingType;
+import org.apache.activemq.artemis.tests.integration.IntegrationTestLogger;
+import org.junit.Before;
+import org.junit.Test;
+
+public class RemoteBindingWithoutLoadBalancingTest extends ClusterTestBase
{
+
+ private static final IntegrationTestLogger log =
IntegrationTestLogger.LOGGER;
+
+ @Override
+ @Before
+ public void setUp() throws Exception {
+ super.setUp();
+
+ setupServers();
+ }
+
+ protected boolean isNetty() {
+ return false;
+ }
+
+ /**
+* It's possible that when a cluster has disabled message load
balancing then a message
+* sent to a node that only has a corresponding remote queue binding
will trigger a
+* stack overflow.
+*/
+ @Test
+ public void testStackOverflow() throws Exception {
+ setupCluster();
+
+ startServers();
+
+ setupSessionFactory(0, isNetty());
+ setupSessionFactory(1, isNetty());
+
+ createQueue(0, "queues.testaddress", "queue0", null, false);
+
+ waitForBindings(0, "queues.testaddress", 1, 0, true);
+
+ waitForBindings(1, "queues.testaddress", 1, 0, false);
+
+ send(1, "queues.testaddress", 1, false, null);
--- End diff --
@michalxo, my guess is that you're sending the message with a JMS client.
The test is using the core API which doesn't auto-create queues.
---
[GitHub] activemq-artemis issue #2370: ARTEMIS-2125 Queue preference changes to displ...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/2370 @tadayosi I've opened https://github.com/apache/activemq-artemis/pull/2373 to address this one too (using your commit :)) ---
[GitHub] activemq-artemis issue #2373: ARTEMIS-2125 Queue preference changes to displ...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/2373 @tadayosi Please take a look if it is similar to what you have done on queues...and thanks for the PR :+1: ---
[GitHub] activemq-artemis pull request #2373: ARTEMIS-2125 Queue preference changes t...
GitHub user franz1981 opened a pull request: https://github.com/apache/activemq-artemis/pull/2373 ARTEMIS-2125 Queue preference changes to display columns not persistent through page refresh You can merge this pull request into a Git repository by running: $ git pull https://github.com/franz1981/activemq-artemis ARTEMIS-2125 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/activemq-artemis/pull/2373.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #2373 commit fe971a7a976cf24c4fe1c05b976099e509ecde53 Author: Tadayoshi Sato Date: 2018-10-15T05:38:43Z ARTEMIS-2125 Queue preference changes to display columns not persistent through page refresh commit c4dd09cb8d41589a2156068bc0881df49e1149b3 Author: Francesco Nigro Date: 2018-10-16T11:28:14Z ARTEMIS-2125 Tabs pref changes to display columns not persisted through page refresh ---
[GitHub] activemq-artemis pull request #2371: ARTEMIS-2097 Pause and Block Producers
Github user gaohoward closed the pull request at: https://github.com/apache/activemq-artemis/pull/2371 ---
[GitHub] activemq-artemis issue #2371: ARTEMIS-2097 Pause and Block Producers
Github user gaohoward commented on the issue: https://github.com/apache/activemq-artemis/pull/2371 closing it for now, it needs more discussion. ---
Re: [discussion] About blocking producers
There may be two different use cases here; 1) the pause/resume use case, where it makes some sense to leave producers active stop reading/deny credit/buffer etc; the corollary to the pause consumers 2) the take down for maintenance case, where it would be nice to drain the broker. In the past, users have tackled the "maintenance" use case with separate acceptors, one for producers and one for consumers. with the ability to disable an acceptor via a management operation. That is not ideal b/c it requires cooperation with the applications to use the correct acceptors etc. However it is a coarse grained option that is a step to shutdown, a one way step that won't be undone via resume. That can more easily be implemented with a management op that kills off producers and blocks all send operations with some deny interceptor (along the lines of the authorisation interceptors). something like shutdownSenders If the use case is more like shutdownSenders keeping it coarse (on or off for the entire server) will be best. For the pause/resume case it gets tricky; - for example if the same connection is used for producing and consuming, any exception can be fatal for the connection, causing retries etc and looping. - blocking will only work for sync send operations and async operations may accumulate. It may require protocol specific smarts. - same for of credit, only where flow control is respected by the client. I guess my point is that pause/resume may not be the way to go if the use case is actually shut down for maintenance. On Tue, 16 Oct 2018 at 04:19 Howard Gao wrote: > Hi guys, > > I'm working on this story and I'd like to hear your thoughts. > > The Jira: https://issues.apache.org/jira/browse/ARTEMIS-2097 > > This is to provide a functionality of the broker to block on producers so > that user can consume all the existing messages (for maintaining their > servers for example), regardless of how the address setting's address full > policy is configured. The unblock > functionality should also be provided to complete this feature. > > Here is my current implementation: > https://github.com/apache/activemq-artemis/pull/2371 > > The idea is to keep a list of blocked/unblock addresses set by the user. > Any incoming messages will be checked against the list and if its address > is blocked, it will be rejected and an exception will be thrown to the > clients. It works for all types of clients (core, openwire, jms, mqtt > etc). > > Any comments are welcome. > > Thanks > Howard >
[GitHub] activemq-artemis pull request #2345: ARTEMIS-2108 Potential StackOverflowErr...
Github user michalxo commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2345#discussion_r225473981
--- Diff:
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/distribution/RemoteBindingWithoutLoadBalancingTest.java
---
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.cluster.distribution;
+
+import
org.apache.activemq.artemis.core.server.cluster.impl.MessageLoadBalancingType;
+import org.apache.activemq.artemis.tests.integration.IntegrationTestLogger;
+import org.junit.Before;
+import org.junit.Test;
+
+public class RemoteBindingWithoutLoadBalancingTest extends ClusterTestBase
{
+
+ private static final IntegrationTestLogger log =
IntegrationTestLogger.LOGGER;
+
+ @Override
+ @Before
+ public void setUp() throws Exception {
+ super.setUp();
+
+ setupServers();
+ }
+
+ protected boolean isNetty() {
+ return false;
+ }
+
+ /**
+* It's possible that when a cluster has disabled message load
balancing then a message
+* sent to a node that only has a corresponding remote queue binding
will trigger a
+* stack overflow.
+*/
+ @Test
+ public void testStackOverflow() throws Exception {
+ setupCluster();
+
+ startServers();
+
+ setupSessionFactory(0, isNetty());
+ setupSessionFactory(1, isNetty());
+
+ createQueue(0, "queues.testaddress", "queue0", null, false);
+
+ waitForBindings(0, "queues.testaddress", 1, 0, true);
+
+ waitForBindings(1, "queues.testaddress", 1, 0, false);
+
+ send(1, "queues.testaddress", 1, false, null);
--- End diff --
Is this really reproducing the issue @jbertram ?
In my testing, when I send message to broker2, the queue is auto-created,
so previously only remote bindings will change to local bindings, OR
message is refused, due to not-existing queue.
---
[GitHub] activemq-artemis issue #2362: ARTEMIS-2117 Add custom LVQ Key and Non Destru...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/2362 will merge if no comments ---
[GitHub] activemq-artemis pull request #2369: ARTEMIS-2123 Paging not stopped if ther...
Github user franz1981 commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2369#discussion_r225419711
--- Diff:
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageCursorProviderImpl.java
---
@@ -599,6 +600,18 @@ private long checkMinPage(Collection
cursorList) {
}
+ private void deliverIfNecessary(Collection
cursorList, long minPage) {
+ for (PageSubscription cursor : cursorList) {
+ long firstPage = cursor.getFirstPage();
+ if (firstPage == minPage) {
+if (cursor.getQueue().getMessageCount() == 0 &&
cursorList.size() > 1) {
+ cursor.getQueue().deliverAsync();
+ break;
--- End diff --
The `break` could be taken out, landing into `if (firstPage == minPage) {
..break; }`?
@wy96f @clebertsuconic how many cursors can have `cursor.getFirstPage() ==
minPage`?
---
