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/07/26 21:56:22 UTC
[GitHub] [activemq-artemis] gtully opened a new pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
gtully opened a new pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673
… rather than the address from the queue which may be a wildcard sub and not valid for publishng on, fix and test
--
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
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r677844213
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
This is why for redistribution the queue is used, not the message's address.
--
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
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r681623271
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
would better solution here be then to look up binding by queue name from the queue itself? not by address. As redistribution is at queue level anyhow?
--
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
[GitHub] [activemq-artemis] gtully commented on pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
gtully commented on pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#issuecomment-887054367
I am thinking there must be some tests that depend on the old behaviour, lets see. sanity checks on the ones that appear relevant don't show a culprit so I will await a full suite run to validate.
--
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
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r679327434
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
as far as I know, the redistribution will send to a specific queue.. not a reroute, so Divert should not be in play.
--
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
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r681619593
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
move case is where you move a message using admin function. Or in case of DLQ or Expiry Queues, where message is auto moved into corresponding defined dlq or expiry 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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
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 #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r699429479
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
i didnt see any comment on this....
--
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
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r677840524
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
+ Bindings bindings = addressManager.getBindingsForRoutingAddress(SimpleString.toSimpleString(message.getAddress()));
if (bindings != null && bindings.allowRedistribute()) {
// We have to copy the message and store it separately, otherwise we may lose remote bindings in case of restart before the message
// arrived the target node
// as described on https://issues.jboss.org/browse/JBPAPP-6130
Message copyRedistribute = message.copy(storageManager.generateID());
- copyRedistribute.setAddress(originatingQueue.getAddress());
-
- if (tx != null) {
Review comment:
Isnt this re-introducing an old bug that left large messages hanging around when transactional.
--
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
[GitHub] [activemq-artemis] gtully commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
gtully commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r679211135
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
the divert case is ok, a divert copies and sets the target address on the copy. What is the move case?
The scenario where it breaks is where the queue is a wildcard queue, not a routable address. I am really reverting a change from 3years ago that looks like it was done for the wrong reason, prefixes being left on a message.
see: https://github.com/apache/activemq-artemis/commit/f09bde07dfa56e2c65ccf04adf8e158003fd673b
--
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
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r677841526
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
+ Bindings bindings = addressManager.getBindingsForRoutingAddress(SimpleString.toSimpleString(message.getAddress()));
Review comment:
Why creating simple string of the address why not call - getAddressSimpleString
--
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
[GitHub] [activemq-artemis] gtully commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
gtully commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r678087060
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
+ Bindings bindings = addressManager.getBindingsForRoutingAddress(SimpleString.toSimpleString(message.getAddress()));
Review comment:
good call, will fix
--
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
[GitHub] [activemq-artemis] gtully merged pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
gtully merged pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673
--
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
[GitHub] [activemq-artemis] gtully commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
gtully commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r700365918
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
I don't think that will work with wildcards, b/c the queue can be bound multiple times, depending on what addresses it matches. Also a remote a/+ is a match for a local a/#, both match a message published to address a.b
--
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
[GitHub] [activemq-artemis] gtully commented on pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
gtully commented on pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#issuecomment-887454499
there was one failure in redistribution with prefixes, which pointed to the cause of this problem. https://github.com/apache/activemq-artemis/pull/1782
re working the fix to update the message address once the routing is transformed seems correct. It keeps the prefix logic confined to the acceptor/session that produces the message. I notice that there is already some mangling going on for legacy clients in relation to prefixes.
Of note in the test on redistribution and prefixes, the consumer is on a "non prefixed" queue, the old behaviour would have an address with a prefix on messages on that queue, which seems odd.
--
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
[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r699429479
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
i didnt see any comment on this....
--
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
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r677842252
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
Isnt this change breaking use case where a message is moved from its original address to another queue and address, from which it then needs to re-distribute.....
--
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
[GitHub] [activemq-artemis] gtully commented on pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
gtully commented on pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#issuecomment-913511083
rebased
--
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
[GitHub] [activemq-artemis] Elyytscha commented on pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
Elyytscha commented on pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#issuecomment-888368841
> there may be some issue though, I see an intermittent failure in org.apache.activemq.artemis.tests.integration.amqp.AmqpBridgeClusterRedistributionTest#testSendMessageToBroker0GetFromBroker2 that I need to investigate. It fails most of the time.
i have a compiled artemis.jar with the latest commit from this branch, does it make sense to test already or should i wait until you had time to investigate and maybe push some changes?
--
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
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r677842252
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
This change breaking use case where a message is moved from its original address to another queue and address, from which it then needs to re-distribute..... e.g. where manually re-routed, or diverts, or auto dlq/expiries are used.... and they have redistribution on them,.
--
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
[GitHub] [activemq-artemis] Elyytscha commented on pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
Elyytscha commented on pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#issuecomment-887461312
if you can link the docs where its explained how to build artemis from your fork, i would be willing to test this on my local machine, we have written a docker-compose based test suite which spins up an artemis cluster and which is able to reproduce this error. so we would just need a compiled artemis.jar from your fork to test this out on our side i think.
best regards and thanks for working on this :)
--
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
[GitHub] [activemq-artemis] Elyytscha commented on pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
Elyytscha commented on pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#issuecomment-889766653
Hello,
we tested already, but i'm not sure if we did everything correct.
redistribution on the same node is now okay with mqtt and clean session false,
when you subscribe to a wildcard topic with clean session false, then disconnect, then send 2 messages on node a and node b to a topic in this wildcard range with different publisher, then reconnect the client, the messages are appearing with their original topic, not with a wildcard anymore.
but we have some weird behavior with the management console and with same steps above but connecting to node b instead of node a where subscriber was connected before, it does not work, there are generally no messages arriving. the weird error we see and why we cant check the settings in the webconsole is actually:
```
artemis1_1 | 2021-07-29 20:07:33,496 WARN [io.hawt.system.Authenticator] Login failed due to: No LoginModules configured for activemq-management-management-management-management-management-management-management-management
artemis1_1 | 2021-07-29 20:07:34,026 WARN [io.hawt.system.Authenticator] Login failed due to: No LoginModules configured for activemq-management-management-management-management-management-management-management-management
artemis1_1 | 2021-07-29 20:07:35,454 WARN [io.hawt.system.Authenticator] Login failed due to: No LoginModules configured for activemq-management-management-management-management-management-management-management-management
```
with this login.config:
```
activemq {
org.apache.activemq.artemis.spi.core.security.jaas.GuestLoginModule sufficient
debug=true
org.apache.activemq.jaas.guest.user="guest"
org.apache.activemq.jaas.guest.role="guest";
};
activemq-management {
org.apache.activemq.artemis.spi.core.security.jaas.PropertiesLoginModule sufficient
debug=true
org.apache.activemq.jaas.properties.user="artemis-users.properties"
org.apache.activemq.jaas.properties.role="artemis-roles.properties";
};
```
but i can't imagine how that should be related to this pullrequest, so i think we have some errors in the setup or something has changed since activemq 2.16 which we used before.. so i think we have some configuration error, we are investigating this..
best regards
--
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
[GitHub] [activemq-artemis] gtully commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
gtully commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r700381258
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
I just peeked at queueImpl.move (used by dlq and expiry), it will reset the message address. so a DLQ address can be redistributed ok. Is that your concern?
--
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
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r681623271
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
would better solution here be then to look up binding by queue name? not by address. As redistribution is at queue level anyhow?
--
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
[GitHub] [activemq-artemis] gtully commented on pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
gtully commented on pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#issuecomment-888408629
@Elyytscha I think go ahead and verify on your end. From what I have been able to understand so far the intermittent failure in org.apache.activemq.artemis.tests.integration.amqp.AmqpBridgeClusterRedistributionTest#testSendMessageToBroker0GetFromBroker2 seems to be timing related. thanks for the help :-)
--
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
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r681623271
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
would better solution here be then to look up binding by name? not by address. As redistribution is at queue level anyhow?
--
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
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r677842252
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
Is this not breaking change for use case where a message is moved from its original address to another queue and address, from which it then needs to re-distribute..... e.g. where manually re-routed, or diverts, or auto dlq/expiries are used.... and they have redistribution on them.
--
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
[GitHub] [activemq-artemis] Elyytscha commented on pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
Elyytscha commented on pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#issuecomment-891951235
Hello
we can confirm that with this pullrequest our problem is solved:
we can now do the following in an artemis cluster with mqtt with clean session false in mosquitto-client:
subscribe suscriber1 to node1, disconnect subscriber1 from node1 , publish message test1 on node1 with publisher1 and test2 on node2 with publisher2, connect subscriber1 to node2, topic arrives correctly:
```
Client subscriber1 sending CONNECT
Client subscriber1 received CONNACK
Client subscriber1 sending SUBSCRIBE (Mid: 1, Topic: /drivers/+/services, QoS: 0)
Client subscriber1 received PUBLISH (d0, q0, r0, m0, '/drivers/user-x/services', ... (5 bytes))
test1
Client subscriber1 received PUBLISH (d0, q0, r0, m0, '/drivers/user-x/services', ... (5 bytes))
test2
Client subscriber1 received SUBACK
Subscribed (mid: 1): 0
```
without this pullrequest (plain artemis 2.18) the same test looks like this:
```
Client subscriber1 sending CONNECT
Client subscriber1 received CONNACK
Client subscriber1 sending SUBSCRIBE (Mid: 1, Topic: /drivers/+/services, QoS: 0)
Client subscriber1 received PUBLISH (d0, q0, r0, m0, '/drivers/+services', ... (5 bytes))
test1
Client subscriber1 received PUBLISH (d0, q0, r0, m0, '/drivers/+/services', ... (5 bytes))
test2
Client subscriber1 received SUBACK
Subscribed (mid: 1): 0
```
--
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
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r677844213
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
This is why for redistribution the queue is used, not the message's address.
--
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
[GitHub] [activemq-artemis] Elyytscha edited a comment on pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
Elyytscha edited a comment on pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#issuecomment-891951235
Hello
we can confirm that with this pullrequest our problem is solved:
we can now do the following in an artemis cluster with mqtt with clean session false in mosquitto-client:
subscribe suscriber1 to node1, disconnect subscriber1 from node1 , publish message test1 on node1 with publisher1 and test2 on node2 with publisher2, connect subscriber1 to node2, topic arrives correctly:
```
mosquitto_sub -i subscriber1 -h artemis-node1 -p 1883 -t 'drivers/+/services' -d -c
# CTRL+C for disconnect
mosquitto_pub -i publisher1 -h artemis-node1 -p 1883 -t drivers/user-x/services -m test1 -d
mosquitto_pub -i publisher2 -h artemis-node2 -p 1883 -t drivers/user-x/services -m test2 -d
mosquitto_sub -i subscriber1 -h artemis-node2 -p 1883 -t 'drivers/+/services' -d -c
Client subscriber1 sending CONNECT
Client subscriber1 received CONNACK
Client subscriber1 sending SUBSCRIBE (Mid: 1, Topic: drivers/+/services, QoS: 0)
Client subscriber1 received PUBLISH (d0, q0, r0, m0, 'drivers/user-x/services', ... (5 bytes))
test1
Client subscriber1 received PUBLISH (d0, q0, r0, m0, 'drivers/user-x/services', ... (5 bytes))
test2
Client subscriber1 received SUBACK
Subscribed (mid: 1): 0
```
without this pullrequest (plain artemis 2.18) the same test looks like this:
```
mosquitto_sub -i subscriber1 -h artemis-node1 -p 1883 -t 'drivers/+/services' -d -c
# CTRL+C for disconnect
mosquitto_pub -i publisher1 -h artemis-node1 -p 1883 -t drivers/user-x/services -m test1 -d
mosquitto_pub -i publisher2 -h artemis-node2 -p 1883 -t drivers/user-x/services -m test2 -d
mosquitto_sub -i subscriber1 -h artemis-node2 -p 1883 -t 'drivers/+/services' -d -c
Client subscriber1 sending CONNECT
Client subscriber1 received CONNACK
Client subscriber1 sending SUBSCRIBE (Mid: 1, Topic: drivers/+/services, QoS: 0)
Client subscriber1 received PUBLISH (d0, q0, r0, m0, 'drivers/+/services', ... (5 bytes))
test1
Client subscriber1 received PUBLISH (d0, q0, r0, m0, 'drivers/+/services', ... (5 bytes))
test2
Client subscriber1 received SUBACK
Subscribed (mid: 1): 0
```
--
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
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r677842252
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
This change breaking use case where a message is moved from its original address to another queue and address, from which it then needs to re-distribute..... e.g. where manually re-routed, or diverts are used....
--
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
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r677842252
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
Is this not breaking change for use case where a message is moved from its original address (which may have redistribute set to OFF) to another address/queue and address (which has different settings e.g. redistribute ON_DEMAND), from which it then needs to re-distribute..... e.g. where manually re-routed, or diverts, or auto dlq/expiries are used.... and they have redistribution on them.
--
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
[GitHub] [activemq-artemis] gtully commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
gtully commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r678107566
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
I don't think so, redistribution (when enabled via a redistribution-delay != -1 is redistributing the message. Where it was stuck is just an artefact of where it is stuck. The message is still the message.
the wrangling of the address seems to come from a bad fix for a problem with prefixes left in 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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] Elyytscha edited a comment on pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
Elyytscha edited a comment on pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#issuecomment-891951235
Hello
we can confirm that with this pullrequest our problem is solved:
we can now do the following in an artemis cluster with mqtt with clean session false in mosquitto-client:
subscribe suscriber1 to node1, disconnect subscriber1 from node1 , publish message test1 on node1 with publisher1 and test2 on node2 with publisher2, connect subscriber1 to node2, topic arrives correctly:
```
mosquitto_sub -i subscriber1 -h artemis-node2 -p 1883 -t 'drivers/+/services' -d -c
Client subscriber1 sending CONNECT
Client subscriber1 received CONNACK
Client subscriber1 sending SUBSCRIBE (Mid: 1, Topic: drivers/+/services, QoS: 0)
Client subscriber1 received PUBLISH (d0, q0, r0, m0, 'drivers/user-x/services', ... (5 bytes))
test1
Client subscriber1 received PUBLISH (d0, q0, r0, m0, 'drivers/user-x/services', ... (5 bytes))
test2
Client subscriber1 received SUBACK
Subscribed (mid: 1): 0
```
without this pullrequest (plain artemis 2.18) the same test looks like this:
```
mosquitto_sub -i subscriber1 -h artemis-node2 -p 1883 -t 'drivers/+/services' -d -c
Client subscriber1 sending CONNECT
Client subscriber1 received CONNACK
Client subscriber1 sending SUBSCRIBE (Mid: 1, Topic: drivers/+/services, QoS: 0)
Client subscriber1 received PUBLISH (d0, q0, r0, m0, 'drivers/+/services', ... (5 bytes))
test1
Client subscriber1 received PUBLISH (d0, q0, r0, m0, 'drivers/+/services', ... (5 bytes))
test2
Client subscriber1 received SUBACK
Subscribed (mid: 1): 0
```
--
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
[GitHub] [activemq-artemis] gtully commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
gtully commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r678086641
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
+ Bindings bindings = addressManager.getBindingsForRoutingAddress(SimpleString.toSimpleString(message.getAddress()));
if (bindings != null && bindings.allowRedistribute()) {
// We have to copy the message and store it separately, otherwise we may lose remote bindings in case of restart before the message
// arrived the target node
// as described on https://issues.jboss.org/browse/JBPAPP-6130
Message copyRedistribute = message.copy(storageManager.generateID());
- copyRedistribute.setAddress(originatingQueue.getAddress());
-
- if (tx != null) {
Review comment:
no. that is dead code.
--
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
[GitHub] [activemq-artemis] gtully commented on pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
gtully commented on pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#issuecomment-887475349
clone https://github.com/gtully/activemq-artemis/tree/ARTEMIS-3223
set JAVA_HOME=..
mvn clean install -Pdev -DfailIfNoTests=false -Dtest=MqttClusterWildcardTest
in a few minutes you will have a distribution in:
artemis-distribution/target/apache-artemis-2.18.0-SNAPSHOT-bin
there are more instructions at: https://activemq.apache.org/components/artemis/documentation/2.0.0/hacking-guide/tests.html but I think you will be good to go.
--
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
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r677844213
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
This is why for redistribution the queue is used, not the message's original address.
--
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
[GitHub] [activemq-artemis] gtully commented on pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
gtully commented on pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#issuecomment-912570916
finally got to the source of the failure in org.apache.activemq.artemis.tests.integration.amqp.AmqpBridgeClusterRedistributionTest#testSendMessageToBroker0GetFromBroker2
the test sometimes uses redistribution, depending if the bridge gets setup on time. When it did the forward errored out silently... fixed that... but the root case was the need to keep the setAddress on the copy.
--
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
[GitHub] [activemq-artemis] gtully commented on pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
gtully commented on pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#issuecomment-888136234
there may be some issue though, I see an intermittent failure in org.apache.activemq.artemis.tests.integration.amqp.AmqpBridgeClusterRedistributionTest#testSendMessageToBroker0GetFromBroker2 that I need to investigate. It fails most of the time.
--
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
[GitHub] [activemq-artemis] gtully commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
gtully commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r679338994
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
the crux of the problem here is that the queue address contains wildcards and those are not valid in for finding matching bindings or as the message address. you cannot send to a.#, you can only consume a.b from a.#
--
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
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r677842252
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
Is this not breaking change for use case where a message is moved from its original address to another queue and address, from which it then needs to re-distribute..... e.g. where manually re-routed, or diverts, or auto dlq/expiries are used.... and they have redistribution on them,.
--
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
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3673: ARTEMIS-3223 - ensure distribution uses the address from the message,…
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3673:
URL: https://github.com/apache/activemq-artemis/pull/3673#discussion_r678621342
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -1289,29 +1289,13 @@ public MessageReference reload(final Message message, final Queue queue, final T
public Pair<RoutingContext, Message> redistribute(final Message message,
final Queue originatingQueue,
final Transaction tx) throws Exception {
- Bindings bindings = addressManager.getBindingsForRoutingAddress(originatingQueue.getAddress());
Review comment:
But the point is the address could be different if the message has been diverted or moved within the broker. Redistribution should use the queue its being Redistributed from not the messages set value. As Redistribution is at the queue level.
--
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