You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by gaohoward <gi...@git.apache.org> on 2018/10/15 08:17:59 UTC
[GitHub] activemq-artemis pull request #2371: ARTEMIS-2097 Pause and Block Producers
GitHub user gaohoward opened a pull request:
https://github.com/apache/activemq-artemis/pull/2371
ARTEMIS-2097 Pause and Block Producers
Added new methods to AddressControl and ServerControl to allow users
to block sending on addresses. Once blocked, any messages sending
to the blocked addresses will get rejected with an exception. The
senders can catch the corresponding exception and handle properly.
For example, keep resending or send messages to some alternative
addresses.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gaohoward/activemq-artemis a_2097
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/2371.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 #2371
----
commit 056e09b036ffe4eb0022328a837cfa8ec7c28203
Author: Howard Gao <ho...@...>
Date: 2018-10-15T08:10:31Z
ARTEMIS-2097 Pause and Block Producers
Added new methods to AddressControl and ServerControl to allow users
to block sending on addresses. Once blocked, any messages sending
to the blocked addresses will get rejected with an exception. The
senders can catch the corresponding exception and handle properly.
For example, keep resending or send messages to some alternative
addresses.
----
---
[GitHub] activemq-artemis pull request #2371: ARTEMIS-2097 Pause and Block Producers
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2371#discussion_r225383942
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java ---
@@ -1789,6 +1790,11 @@ public synchronized RoutingStatus doSend(final Transaction tx,
AddressInfo art = getAddressAndRoutingType(new AddressInfo(msg.getAddressSimpleString(), routingType));
+ if (postOffice.isAddressBlocked(msg.getAddressSimpleString())) {
--- End diff --
You mean a formal discussion? I'll send an email for that purpose. I think it not too late. :)
---
[GitHub] activemq-artemis pull request #2371: ARTEMIS-2097 Pause and Block Producers
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2371#discussion_r225353155
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java ---
@@ -1789,6 +1790,11 @@ public synchronized RoutingStatus doSend(final Transaction tx,
AddressInfo art = getAddressAndRoutingType(new AddressInfo(msg.getAddressSimpleString(), routingType));
+ if (postOffice.isAddressBlocked(msg.getAddressSimpleString())) {
--- End diff --
@clebertsuconic reply here.
I can think of 2 difficulties using credits:
1. Some clients (like mqtt, stomp) are not supporting flow control so they can't be blocked using credits.
2. producers are not necessarily bound to one address, they can send messages to different addresses (for example one producer can send one message to address1, and the next one to address2. It it's blocked on address1, the next message also be blocked)
If we simply block the sending threads, it will holding up broker threads (like you said OME). Besides the producer may have call-timeout exception.
By throwing a proper exception we can let client to handle their producer properly without blocking any thing.
---
[GitHub] activemq-artemis pull request #2371: ARTEMIS-2097 Pause and Block Producers
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2371#discussion_r225357936
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java ---
@@ -1789,6 +1790,11 @@ public synchronized RoutingStatus doSend(final Transaction tx,
AddressInfo art = getAddressAndRoutingType(new AddressInfo(msg.getAddressSimpleString(), routingType));
+ if (postOffice.isAddressBlocked(msg.getAddressSimpleString())) {
--- End diff --
We should have discussed this before you coming into implementation.
I don't think this is the right design.
---
[GitHub] activemq-artemis pull request #2371: ARTEMIS-2097 Pause and Block Producers
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2371#discussion_r225165274
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java ---
@@ -1789,6 +1790,11 @@ public synchronized RoutingStatus doSend(final Transaction tx,
AddressInfo art = getAddressAndRoutingType(new AddressInfo(msg.getAddressSimpleString(), routingType));
+ if (postOffice.isAddressBlocked(msg.getAddressSimpleString())) {
--- End diff --
I think the use case here is wrong. With block you just stop sending credits to the client.
This becomes fail at this point.
We have two options:
- If you had previously sent credits to the client.. then you must complete the operations until you have full credits.
- or we could hung to these on the server. But that would have a risk of running OME. So I would say we just stop sending credits.
---
[GitHub] activemq-artemis pull request #2371: ARTEMIS-2097 Pause and Block Producers
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2371#discussion_r225353248
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java ---
@@ -1789,6 +1790,11 @@ public synchronized RoutingStatus doSend(final Transaction tx,
AddressInfo art = getAddressAndRoutingType(new AddressInfo(msg.getAddressSimpleString(), routingType));
+ if (postOffice.isAddressBlocked(msg.getAddressSimpleString())) {
--- End diff --
btw my test missed stomp and mqtt, I'll add them.
---
[GitHub] activemq-artemis issue #2371: ARTEMIS-2097 Pause and Block Producers
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the issue:
https://github.com/apache/activemq-artemis/pull/2371
closing it for now, it needs more discussion.
---
[GitHub] activemq-artemis pull request #2371: ARTEMIS-2097 Pause and Block Producers
Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2371#discussion_r225093373
--- Diff: artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/ActiveMQAddressBlockedException.java ---
@@ -0,0 +1,24 @@
+/*
+ * 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.api.core;
+
+public class ActiveMQAddressBlockedException extends ActiveMQException {
+
+ public ActiveMQAddressBlockedException(String address) {
--- End diff --
To help the JVM to handle this exception like a "common" case (and not exceptional): i suggest to disable the stack trace generation for this exception by adding:
```
@Override
public Throwable fillInStackTrace() {
return this;
}
```
That would allow to create `ActiveMQAddressBlockedException` as a singleton and just reusing it a signal.
I'm assuming that the use case for pause/block producers is making intensive use of this exception as an event that allow recovering and deciding what to do next in the common execution path.
---
[GitHub] activemq-artemis pull request #2371: ARTEMIS-2097 Pause and Block Producers
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward closed the pull request at:
https://github.com/apache/activemq-artemis/pull/2371
---
[GitHub] activemq-artemis pull request #2371: ARTEMIS-2097 Pause and Block Producers
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2371#discussion_r225143852
--- Diff: artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/ActiveMQAddressBlockedException.java ---
@@ -0,0 +1,24 @@
+/*
+ * 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.api.core;
+
+public class ActiveMQAddressBlockedException extends ActiveMQException {
+
+ public ActiveMQAddressBlockedException(String address) {
--- End diff --
Yes that makes sense. We don't need the stacktrace here. I'll add that. thx.
---
[GitHub] activemq-artemis pull request #2371: ARTEMIS-2097 Pause and Block Producers
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2371#discussion_r225164140
--- Diff: artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/ActiveMQAddressBlockedException.java ---
@@ -0,0 +1,24 @@
+/*
+ * 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.api.core;
+
+public class ActiveMQAddressBlockedException extends ActiveMQException {
+
+ public ActiveMQAddressBlockedException(String address) {
--- End diff --
Why do you need a BlockedException? With block you just bock, no exceptions!
---