You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2020/05/09 18:05:28 UTC

[GitHub] [activemq-artemis] Havret opened a new pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN

Havret opened a new pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124


   This is my attempt to address the problem of the auto-creation of queues based on FQQN. The simple scenario seems to work, but some decisions need to be made before I can move forward with this. 
   
   1) There is a problem with the `RoutingType` mismatch. Previously it didn't matter what `RoutingType` receiver tries to attach to, as the implementation assumed pre-existence of the queue. Now the routing type matches, as we need to create a properly configured queue if it doesn't exist. That's why `testQueueConsumerReceiveTopicUsingFQQN
   ` test fails currently. To make it pass I would need to subscribe to a topic instead of a queue. I'm not sure but is this a breaking change or not? 
   2) I'm not sure what is the expected behavior when durability configuration mismatches. The problem is described in a test `testAttachToPreConfiguredNonDurableQueueUsingDurableFQQN
   `. Should we reject the attempt to attach when the pre-configured queue is non-durable and attach request has the source with `TerminusDurability
   ` of `UNSETTLED_STATE` or `CONFIGURATION`? And what about the opposite, pre-configured queue is durable and we received source with `NONE` value for `TerminusDurability`?
   
   I would greatly appreciate your 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633269335


   
   To be honest i would expect for FQQN it to apply all the exact logic thats in there, except the where we call createQueueName you use queueNameToUse if it present instead, that was possibly extracted earlier in the composite address extraction
   
   e.g. line 413
   
                  queue = createQueueName(connection.isUseCoreSubscriptionNaming(), clientId, pubId, shared, global, false);
   
   to 
   
                  queue = queueNameToUse == null ? createQueueName(connection.isUseCoreSubscriptionNaming(), clientId, pubId, shared, global, false) : queueNameToUse;
   
   
   And like wise line 438 extactly the same
   
   I wouldnt expect changes anywhere else to achieve this tbh.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN

Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-628310271


   > 
   > 
   > @gemmellr I'm bringing up JMS because JMS-AMQP mapping was the first thing you referred me to. 
   
   I didn't refer you to it. Someone else may have briefly linked to it.
   
   > Speaking in terms of AMQP spec, words like `shared`, `global`, `topic`, and `subscription` doesn't appear there even once, and you keep bringing them up. 
   
   Topic and subscriptions aren't AMQP specific terms (or JMS specific), though are generally understood messaging concepts, but since the spec doesn't deal specifically with queues or topics, merely nodes, plus links, terminus etc, their lack of use isnt surprising to me.
   
   You are correct that the 'shared' and 'global' terms arent in the draft JMS mapping document either, because I have yet to publish the update with them in it. I did only mention those terms after you brought up shared durable subscriptions however, and again I didn't actually direct you to that document.
   
   JMS has no special bearing in this discussion from my view, its purely AMQP from my perspective. 
   
   > I'm not an expert in this area, I've never spoken with the authors of the spec. The only resources I have are spec itself, Artemis Source Code, and its documentation. 
   
   Thats fair enough. You could possibly include in that resource list, discussion from those who people might say are quite familiar with the area, and are indicating having discussed this very specific item with authors on more than one occasion (including yesterday).
   
   > Forgive me my ignorance but if I read in docs for amqp connector that
   > 
   > > If the Terminus Durability is either UNSETTLED_STATE or CONFIGURATION then the queue will be made durable (similar to a JMS durable subscription) (...).
   > 
   > And I see this being used to implement JMS features I interpret it this way.
   > 
   
   Specifically the doc section it appears you may be quoting from is "AMQP and Multicast Addresses (Topics)", i.e its about topics rather than queues, aligning with what I have said.
   
   The broker models its 'topic' subscriptions (i.e consumer link attaching using address of, err, an address ) using a queue that it happens to expose. There is no need for an actual exposed queue here at all, its a topic subscription, the queue is essentially a synthetic creation. It is the link terminus attaching to the topic-like node (so, address), i.e effectively the subscription representation, that is being indicated as durable (and separately marked as never expiring, to ensure its contuing existance) in this case to retain it. This is quite a different situation than if the link terminus were intended for attaching directly to an actual queue.
   
   > Shared Durable Subscription that you're recalling as the only feasible case to use TerminusDurability is not an AMQP feature.
   
   I am not saying (and dont recall ever having done so here) that Shared Durable Subscriptions are the only feasible case for TerminusDurability. I have said topic-like subscriptions of any kind are the main use case for it (along with expiry in some cases). I have said that TerminusDurability isnt the same direct thing as queue durability. The spec doesnt deal with creating them and so doesn't deal with their durability.
   
   Separately though, they are however an AMQP feature - a layered extension feature yes, but a fully negotiated one that any AMQP client or server can use doing regular AMQP interactions, and again already do.
   
   Not really important, but for historic clarity, as it happens shared AMQP subscriptions existed in actual use in non-JMS client and servers long before any work was done in that area for a JMS 2 mapping for use in AMQP JMS clients. That existing behaviour was used as a starting point of reference (and one of many many approaches looked at) and expanded slightly to cover other requirements.
   
   > It's a way that JMS based clients can attach to multicast queues where topic maps to address and subscription name maps to durable queue name.
   
   All AMQP clients or servers attach to 'topic-like' nodes (in this case, the brokers 'addresses') in the same way, not just JMS ones. Again, the queue existing in this scenario is completely a broker implementation detail (an approach that others also choose, admittedly), and it is entirely possible to model that topic subscription without any exposed queue.
   
   
   > The fact that the durable queue is created there, is not an implementation detail from my perspective. When I configure broker I'm not configuring it in terms of topics and subscriptions. When I use FQQN I don't even need to know that such things exist. I have a broker with configured addresses and queues and I want to attach to them. In the ideal world, it would be great if it was possible to do it without the need to pre-configure (for instance to make development easier).
   
   I understand that it might be a nice side effect that you desire in certain cases, but that doesnt mean it isnt an impl detail, or that terminus durability is queue durability.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633293839


   I was editing my comment, as hadn;'t fully finished.
   
   Mappings i would expect from a black box testing:
   
   JmsQueueConsumer (createConsumer(Queue)-> shared durable
   JmsTopicConsumer (createConsumer(Topic)) -> non durable shared
   JmsTopicConsumer (createDurableconsumer(Topic)) -> non shared durable
   
   For nonshared, non durable a user would just not use FQQN.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN

Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-628310487


   The way I know some other servers have facilitated things in this space is to enable pre-defining configuration that will apply to a queue/topic/other when things are being auto created, e.g allowing you to pattern match what configuration that applies when a link attaches to a given address. Possibly much like the the existing address config stuff can pre-configure address behaviour without actual addresses.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633289092


   Just to note and for the record, i am cautious of this change, as the FQQN was added for legacy stuff, making this change could be impacting to those expected behaviours. And building on it further encourages its use to proliferate. 
   
   Unlike stomp, which you bring up, AMQP has a very rich messaging structure that supports this, as such its not like comparing stomp or openwire which are limited, in this area.
   
   Personally i think for any higher level AMQP clients (e.g JMS/NMS) they would be better to expend time into adding support for the equivalent of jms shared sub in their abstractions, , so in case of NMS that would be to get that updated to  similar spec level with JMS 2.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret commented on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN

Posted by GitBox <gi...@apache.org>.
Havret commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633233631


   @jbertram, @gemmellr 
   
   So after all it wasn't too productive discussion. I debugged the code, and apparently methods `createSharedDurableQueue` and `createUnsharedDurableQueue` don't explicitly create durable queues. :) The queues are durable by default if it's not specified otherwise. So if drop "setDurable" bits, FQQN queues will be created as durable which makes sense, and doesn't break anything.
   
   However, there are still some things that need to be cleared up regarding my first question. 
   
   > There is a problem with the RoutingType mismatch. Previously it didn't matter what RoutingType receiver tries to attach to, as the implementation assumed pre-existence of the queue. Now the routing type matters, as we need to create a properly configured queue if it doesn't exist. That's why testQueueConsumerReceiveTopicUsingFQQN test fails currently. To make it pass I would need to subscribe to a topic instead of a queue. I'm not sure but is this a breaking change or not?
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret edited a comment on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
Havret edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633292814


   So the behavior shouldn't change unless somebody relies on the fact, that the broker returns an error if one tries to attach to the queue that doesn't exist, instead of simply creating it.
   
   > If i was to guess at a default queue type, i would go with a durable shared queue. You could look to make use of ParameterisedAddress like in Core to get extra queue config for a queue off the name, to allow for more configuration to be able to be present in the fqqn address
   
   I think this is the current behavior as the only difference between `createUnsharedDurableQueue` and is `createSharedDurableQueue` is that Unshared set consumer limit to 1. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-718980203


   I would like to -1000 on this.
   
   it's already complicated having auto-creating and routing messages.
   
   Doing this kind of thing would require doing a lot of computation on queue adding. I don't really think we should use FQQN on auto-create.
   
   quite the opposite you should not be using auto-creatin at all on a production system IMHO.
   
   If you do, use it for simple cases.. this is definitely not a simple case and it would induce a lot of calculations during simple message routes on regular cases.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelpearce-gain commented on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-634306475


   If you look at FQQNOpenWireTest youll notice some existing cross protocol tests, you will want to add AMQP to that, and enhance for any extra cases, to ensure behaviour across JMS is aligned.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
Havret edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633268105


   So the routing type is calculated there multiple times, using slightly different variations of the same logic. It's pretty messy tbh, maybe it would be a good idea to unify this, and extract one method to determine routing type. 
   
   The same story with fields `routingTypeToUse` and `multicast` it is basically the same thing, isn't it? 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633280730


   For AMQP clients they should really be using correct structures of AMQP not coding to FQQN. Its intent was solely for JMS legacy workaround where client has not yet upgraded to JMS 2 which supports shared subs etc. And in JMS 2.0 Amqp client like qpid they correctly set the amqp structures.
   
   E.g. in your case such where NMS doesnt yet support 2.0 (wip) then youre restricted to fqqn for shared subs. Obviously the whole issue is removed when you get NMS 2.0 upto 2.p spec.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
Havret edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633278664


   So if I make these changes, it will work partially for multicast routing type (but all queues will be created using `createUnsharedDurableQueue` method. Not sure if this is desired. And it doesn't work at all for anycast, because of this if:
   
   ![image](https://user-images.githubusercontent.com/9103861/82762546-e6a5c180-9e01-11ea-9733-e56794a18a40.png)
   
   Any suggestions? 
   
   > For anycast i wouldnt even bother, the anycast queue is expected to have been created when the queue address is, for JMS semantics anyhow. (E.g. the original remit of FQQN was allowing a JMS queue consumer bind to a mutlicat queue, to get around issues where a JMS client was still at 1.1 spec without shared sub support )
   
   But we would end up with the implementation that's not in sync with what's available for instance for STOMP. The whole point of this PR was to make it right, and easier to utilize for non-JMS AMQP based clients. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret commented on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
Havret commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633292814


   So the behavior shouldn't change unless somebody relies on the fact, that the broker returns an error if one tries to attach to the queue that doesn't exist, instead of simply creating it.
   
   > If i was to guess at a default queue type, i would go with a durable shared queue. You could look to make use of ParameterisedAddress like in Core to get extra queue config for a queue off the name, to allow for more configuration to be able to be present in the fqqn address
   
   So I think this is the current behavior as the only difference between `createUnsharedDurableQueue` and is `createSharedDurableQueue` is that Unshared set consumer limit to 1. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret edited a comment on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
Havret edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-634267603


   I'm not sure if mixing JMS concepts into this is the best idea. Determining whether a queue is durable/non-durable or shared/non-shared based on routing type seems to me pretty non-intuitive.
   
   Moreover, the way that JMS differentiate `createDurableconsumer(Topic)` and `createConsumer(Topic)` is based on `TerminusDurability` and `TerminusExpiryPolicy`. As @gemmellr  tried to explain to me throughout the big chunk of this thread, this property cannot be used to determine whether a queue is durable or not. 
   
   In the ideal world, I would just use `TerminusDurability` and `shared` capability. Having these two properties with FQQN, would make it possible to fully leverage Artemis Address Model in a dead-simple way:
   
   | Address   | Queue  | Routing Type  | Durable | Shared|amqp address  | capabilities  | durable |
   |---|---|---|---|---|---|---|---|
   | a1  | q1  | Multicast  | true  | true  | a1::q1 | shared, topic | 2|
   | a1  | q1  | Anycast| true  | true  | a1::q1 | shared, queue | 2|
   | a1  | q1  | Multicast  | true  | false| a1::q1 | topic | 2|
   | a1  | q1  | Anycast| true  | false  | a1::q1 | queue | 2|
   | a1  | q1  | Multicast  | false | true  | a1::q1 | shared, topic | 0 |
   | a1  | q1  | Anycast| true  | false  | a1::q1 | shared, queue | 0 |
   | a1  | q1  | Multicast  | false  | false| a1::q1 | topic | 0 |
   | a1  | q1  | Anycast| false  | false  | a1::q1 | queue | 0 |
   
   If we create a queue based on FQQN, only if the queue doesn't exist, we wouldn't break any existing users, as they are using FQQN to attach to pre-configured queues anyway. 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633274657


   But thats just original logic added for FQQN, so just change line 515 e.g. this
   
            if (!result.isExists()) {
               throw new ActiveMQAMQPNotFoundException("Queue: '" + queueName + "' does not exist");
   
   to return null, instead of throw exception (as like for non fqqn)
   
   do keep the rest of it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633280730


   For AMQP clients they should really be using correct structures of AMQP not coding to FQQN. Its intent was solely for JMS legacy workaround where client has not yet upgraded to JMS 2 which supports shared subs etc. And in JMS 2.0 Amqp client like qpid they correctly set the amqp structures.
   
   E.g. in your case such where NMS doesnt yet support 2.0 (wip) then youre restricted to fqqn for shared subs. Obviously the whole issue is removed when you get NMS 2.0 upto 2.0 spec. 
   
   As such i would just assume if fqqn and queue not found it is a multicast 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633274657


   But thats just original logic added for FQQN, getMatchingQueue, so just change line 515 e.g. this
   
            if (!result.isExists()) {
               throw new ActiveMQAMQPNotFoundException("Queue: '" + queueName + "' does not exist");
   
   to return null, instead of throw exception (as like for non fqqn)
   
   do keep the rest of it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633289092


   Just to note and for the record, i am cautious of this change, as the FQQN was added for legacy stuff, making this change could be impacting to those expected behaviours. And building on it further encourages its use to proliferate. 
   
   Unlike stomp, which you bring up, AMQP has a very rich messaging structure that supports this, as such its not like comparing stomp or openwire which are limited, in this area.
   
   Personally i think for any higher level AMQP clients (e.g JMS/NMS) they would be better to expend time into adding support for the equivalent of jms shared sub in their abstractions, , so in case of NMS that would be to get that updated to  similar spec level with JMS 2.0
   
   I guess mapping would be for JMS 1.1 for fqqn genreated depending on the consumer the client made in jms api.
   
   JmsQueueConsumer -> shareddurable
   JmsTopicConsumer -> non durable
   JmsTopicConsumer (createDurableconsumer) -> non shared durable
   
   I dont think its possible to map to autocreate a shared non durable, when presented fqqn with jms 1.1
   
   You could  look to make use of ParameterisedAddress (re use, like CompositeAddress is) like in Core to get extra queue config for a queue off the name, to allow for more configuration to be able to be present in the fqqn 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633280730


   For AMQP clients they should really be using correct structures of AMQP not coding to FQQN. Its intent was solely for JMS legacy workaround where client has not yet upgraded to JMS 2 which supports shared subs etc. And in JMS 2.0 Amqp client like qpid they correctly set the amqp structures. This was focussed on jms usecases for non jms usecase in amqp, then should advocating using the correct amqp structures.
   
   E.g. in your case such where NMS doesnt yet support 2.0 (wip) then youre restricted to fqqn for shared subs. Obviously the whole issue is removed when you get NMS 2.0 upto 2.0 spec. 
   
   
   
   As such i would just assume in code that if fqqn and queue not found it is a multicast you want to create on
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633274657


   For anycast i wouldnt even bother, the anycast queue is expected to have been created when the queue address is, for JMS semantics anyhow.
   
   But thats just original logic added for FQQN, getMatchingQueue, so just change line 515 e.g. this
   
            if (!result.isExists()) {
               throw new ActiveMQAMQPNotFoundException("Queue: '" + queueName + "' does not exist");
   
   to return null, instead of throw exception (as like for non fqqn)
   
   do keep the rest of it.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633289092


   Just to note and for the record, i am cautious of this change, as the FQQN was added for legacy stuff, making this change could be impacting to those expected behaviours. And building on it further encourages its use to proliferate. 
   
   Unlike stomp, which you bring up, AMQP has a very rich messaging structure that supports this, as such its not like comparing stomp or openwire which are limited, in this area.
   
   Personally i think for any higher level AMQP clients (e.g JMS/NMS) they would be better to expend time into adding support for the equivalent of jms shared sub in their abstractions, , so in case of NMS that would be to get that updated to  similar spec level with JMS 2.0
   
   
   If i was to guess at a default queue type, i would go with a  durable shared queue. You could  look to make use of ParameterisedAddress like in Core to get extra queue config for a queue off the name


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-634282836






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelpearce-gain edited a comment on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-634306475






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633265597


   First off you shouldn't affect the existing logic, e.g. the original use case which was to allow a fqqn to bind to a queue matching the name ignoring the routing type. E.g. if a fqqn matches an existing queue it should bind to it regardless routingtype.
   
   For stomp how its done is"
   
   A RoutingType is passed to the autoCreateDestinationIfPossible method which is based on the frame's header or destination prefix. If that is null then it uses the default value specified in the address-settings.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633280730


   For AMQP clients they should really be using correct structures of AMQP not coding to FQQN. Its intent was solely for JMS legacy workaround where client has not yet upgraded to JMS 2 which supports shared subs etc. And in JMS 2.0 Amqp client like qpid they correctly set the amqp structures.
   
   E.g. in your case such where NMS doesnt yet support 2.0 (wip) then youre restricted to fqqn for shared subs. Obviously the whole issue is removed when you get NMS 2.0 upto 2.0 spec. 
   
   As such i would just assume if fqqn and queue not found it is a multicast 
   
   
   And here i would just focus on the 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
Havret edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633268105


   So the routing type is calculated there multiple times, using slightly different variations of the same logic. It's pretty messy tbh, maybe it would be a good idea to unify this, and extract one method to determine routing type. 
   
   The same story with fields `routingTypeToUse` and `multicast` it basically the same thing? 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633274657


   For anycast i wouldnt even bother, the anycast queue is expected to have been created when the queue address is, for JMS semantics anyhow. (E.g. the original remit of FQQN was allowing a JMS queue consumer bind to a mutlicat queue, to get around issues where a JMS client was still at 1.1 spec without shared sub support )
   
   re Topic (aka multicast)
   
   Thats just original logic added for FQQN, getMatchingQueue, so just change line 515 e.g. this
   
            if (!result.isExists()) {
               throw new ActiveMQAMQPNotFoundException("Queue: '" + queueName + "' does not exist");
   
   to return null, instead of throw exception (as like for non fqqn)
   
   do keep the rest of it.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633289092


   Just to note and for the record, i am cautious of this change, as the FQQN was added for legacy stuff, making this change could be impacting to those expected behaviours. And building on it further encourages its use to proliferate. 
   
   Unlike stomp, which you bring up, AMQP has a very rich messaging structure that supports this, as such its not like comparing stomp or openwire which are limited, in this area.
   
   Personally i think for any higher level AMQP clients (e.g JMS/NMS) they would be better to expend time into adding support for the equivalent of jms shared sub in their abstractions, , so in case of NMS that would be to get that updated to  similar spec level with JMS 2.0
   
   I guess mapping would be for JMS 1.1 for fqqn genreated
   
   JmsQueueConsumer -> shareddurable
   JmsTopicConsumer -> non durable
   JmsTopicConsumer (createDurableconsumer) -> non shared durable
   
   I dont think its possible to map to autocreate a shared non durable, when presented fqqn with jms 1.1
   
   You could  look to make use of ParameterisedAddress like in Core to get extra queue config for a queue off the name, to allow for more configuration to be able to be present in the fqqn 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN

Posted by GitBox <gi...@apache.org>.
Havret edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-628274586


   @gemmellr I'm bringing up JMS because JMS-AMQP mapping was the first thing you referred me to. Speaking in terms of AMQP spec, words like `shared`, `global`, `topic`, and `subscription` doesn't appear there even once, and you keep bringing them up. I'm not an expert in this area, I've never spoken with the authors of the spec. The only resources I have are spec itself, Artemis Source Code, and its documentation. Forgive me my ignorance but if I read in docs for amqp connector that
   
   > If the Terminus Durability is either UNSETTLED_STATE or CONFIGURATION then the queue will be made durable (similar to a JMS durable subscription) (...).
   
   And I see this being used to implement JMS features I interpret it this way.
   
   Shared Durable Subscription that you're recalling as the only feasible case to use TerminusDurability is not an AMQP feature. It's a way that JMS based clients can attach to multicast queues where topic maps to address and subscription name maps to durable queue name. The fact that the durable queue is created there, is not an implementation detail from my perspective. When I configure broker I'm not configuring it in terms of topics and subscriptions. When I use FQQN I don't even need to know that such things exist. I have a broker with configured addresses and queues and I want to attach to them. In the ideal world, it would be great if it was possible to do it without the need to pre-configure (for instance to make development easier). 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633280730


   For AMQP clients they should really be using correct structures of AMQP not coding to FQQN. Its intent was solely for JMS legacy workaround where client has not yet upgraded to JMS 2 which supports shared subs etc. And in JMS 2.0 Amqp client like qpid they correctly set the amqp structures. This was focussed on jms 1.1 legacy usecases as a bridging mechanism where client upgrade to 2.0 wasnt yet feasible. For non jms usecase in amqp, then should advocating using the correct amqp structures.
   
   E.g. in your case such where NMS doesnt yet support 2.0 (wip) then youre restricted to fqqn for shared subs. Obviously the whole issue is removed when you get NMS 2.0 upto 2.0 spec. 
   
   
   
   As such i would just assume in code that if fqqn and queue not found it is a multicast you want to create on
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret commented on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
Havret commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633272152


   I wish it was that simple. Unfortunately for anycast routing type we don't even get to this line. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633289092


   Just to note and for the record, i am cautious of this change, as the FQQN was added for legacy stuff, making this change could be impacting to those expected behaviours. And building on it further encourages its use to proliferate. 
   
   Unlike stomp, which you bring up, AMQP has a very rich messaging structure that supports this, as such its not like comparing stomp or openwire which are limited, in this area.
   
   Personally i think for any higher level AMQP clients (e.g JMS/NMS) they would be better to expend time into adding support for the equivalent of jms shared sub in their abstractions, , so in case of NMS that would be to get that updated to  similar spec level with JMS 2.0
   
   I guess mapping would be for JMS 1.1 for fqqn genreated depending on the consumer the client made in jms api.
   
   JmsQueueConsumer -> shareddurable
   JmsTopicConsumer -> non durable
   JmsTopicConsumer (createDurableconsumer) -> non shared durable
   
   I dont think its possible to map to autocreate a shared non durable, when presented fqqn with jms 1.1
   
   Def need tests for the above., and all the combinations.
   
   You could  look to make use of ParameterisedAddress (re use, like CompositeAddress is) like in Core to get extra queue config for a queue off the name, to allow for more configuration to be able to be present in the fqqn 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633269335


   To be honest i would expect for FQQN it to apply all the exact logic thats in there, except the where we call createQueueName you use queueNameToUse if it present instead, that was possibly extracted earlier in the composite address extraction
   
   e.g. line 413
   
                  queue = createQueueName(connection.isUseCoreSubscriptionNaming(), clientId, pubId, shared, global, false);
   
   to 
   
                  queue = queueNameToUse == null ? createQueueName(connection.isUseCoreSubscriptionNaming(), clientId, pubId, shared, global, false) : queueNameToUse;
   
   
   And like wise line 438 extactly the same
   
   I wouldnt expect changes anywhere else to achieve what you wanted, e.g. no change to any other class or area of the code base, a very small extra if then else  around the queue name 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633289092


   Just to note and for the record, i am cautious of this change, as the FQQN was added for legacy stuff, making this change could be impacting to those expected behaviours. And building on it further encourages its use to proliferate. 
   
   Unlike stomp, which you bring up, AMQP has a very rich messaging structure that supports this, as such its not like comparing stomp or openwire which are limited, in this area.
   
   Personally i think for any higher level AMQP clients (e.g JMS/NMS) they would be better to expend time into adding support for the equivalent of jms shared sub in their abstractions, , so in case of NMS that would be to get that updated to  similar spec level with JMS 2.0
   
   
   If i was to guess at a default queue type, i would go with a  durable shared queue. You could  look to make use of ParameterisedAddress like in Core to get extra queue config for a queue off the name, to allow for more configuration to be able to be present in the fqqn 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN

Posted by GitBox <gi...@apache.org>.
Havret edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633233631


   @jbertram, @gemmellr 
   
   So after all it wasn't too productive discussion. I debugged the code, and apparently methods `createSharedDurableQueue` and `createUnsharedDurableQueue` don't explicitly create durable queues. :) The queues are durable by default if it's not specified otherwise. So if drop "setDurable" bits, FQQN queues will be created as durable which makes sense, and doesn't break anything.
   
   However, there are still some things that need to be cleared up regarding my first question. 
   
   > There is a problem with the RoutingType mismatch. Previously it didn't matter what RoutingType receiver tries to attach to, as the implementation assumed pre-existence of the queue. Now the routing type matters, as we need to create a properly configured queue if it doesn't exist. That's why testQueueConsumerReceiveTopicUsingFQQN test fails currently. To make it pass I would need to subscribe to a topic instead of a queue. I'm not sure but is this a breaking change or not?
   
   STOMP implementation that @jbertram recently added https://github.com/apache/activemq-artemis/pull/3018 seems to silently ignore passed routing type when the mismatch occurs. Should I do the same here? It would be the safest way I guess, but what do you guys think? 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633289092


   Just to note and for the record, i am cautious of this change, as the FQQN was added for legacy stuff, making this change could be impacting to those expected behaviours. And building on it further encourages its use to proliferate. 
   
   Unlike stomp, which you bring up, AMQP has a very rich messaging structure that supports this, as such its not like comparing stomp or openwire which are limited, in this area.
   
   Personally i think for any higher level AMQP clients (e.g JMS/NMS) they would be better to expend time into adding support for the equivalent of jms shared sub in their abstractions, , so in case of NMS that would be to get that updated to  similar spec level with JMS 2.0
   
   
   If i was to guess at a default queue type, i would go with a  durable shared queue. You could  look to make use of ParameterisedAddress like in Core to get extra config for a queue off the name


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633289092


   Just to note and for the record, i am cautious of this change, as the FQQN was added for legacy stuff, making this change could be impacting to those expected behaviours. And building on it further encourages its use to proliferate. 
   
   Unlike stomp, which you bring up, AMQP has a very rich messaging structure that supports this, as such its not like comparing stomp or openwire which are limited, in this area.
   
   Personally i think for any higher level AMQP clients (e.g JMS/NMS) they would be better to expend time into adding support for the equivalent of jms shared sub in their abstractions, , so in case of NMS that would be to get that updated to  similar spec level with JMS 2.0
   
   I guess mapping would be for JMS 1.1 for fqqn genreated
   
   JmsQueueConsumer -> shareddurable
   JmsTopicConsumer -> non durable
   JmsTopicConsumer (createDurableconsumer) -> non shared durable
   
   I dont think its possible to map to autocreate a shared non durable, when presented fqqn with jms 1.1
   
   You could  look to make use of ParameterisedAddress (re use, like CompositeAddress is) like in Core to get extra queue config for a queue off the name, to allow for more configuration to be able to be present in the fqqn 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret commented on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
Havret commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633278664


   So if I make these changes, it will work partially for multicast routing type (but all queues will be created using `createUnsharedDurableQueue` method. Not sure if this is desired. And it doesn't work at all for anycast, because of this if:
   
   ![image](https://user-images.githubusercontent.com/9103861/82762546-e6a5c180-9e01-11ea-9733-e56794a18a40.png)
   
   Any suggestions? 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
Havret edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633233631


   @jbertram, @gemmellr 
   
   So after all it wasn't too productive discussion. I debugged the code, and apparently methods `createSharedDurableQueue` and `createUnsharedDurableQueue` don't explicitly create durable queues. The queues are durable by default if it's not specified otherwise. So if I drop "setDurable" bits, FQQN queues will be created as durable which makes sense, and doesn't break anything.
   
   However, there are still some things that need to be cleared up regarding my first question. 
   
   > There is a problem with the RoutingType mismatch. Previously it didn't matter what RoutingType receiver tries to attach to, as the implementation assumed pre-existence of the queue. Now the routing type matters, as we need to create a properly configured queue if it doesn't exist. That's why testQueueConsumerReceiveTopicUsingFQQN test fails currently. To make it pass I would need to subscribe to a topic instead of a queue. I'm not sure but is this a breaking change or not?
   
   STOMP implementation that @jbertram recently added https://github.com/apache/activemq-artemis/pull/3018 seems to silently ignore passed routing type when the mismatch occurs. Should I do the same here? It would be the safest way I guess, but what do you guys think? 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633289092


   Just to note and for the record, i am cautious of this change, as the FQQN was added for legacy stuff, making this change could be impacting to those expected behaviours. And building on it further encourages its use to proliferate. 
   
   Unlike stomp, which you bring up, AMQP has a very rich messaging structure that supports this, as such its not like comparing stomp or openwire which are limited, in this area.
   
   Personally i think for any higher level AMQP clients (e.g JMS/NMS) they would be better to expend time into adding support for the equivalent of jms shared sub in their abstractions, , so in case of NMS that would be to get that updated to  similar spec level with JMS 2.0
   
   I guess mapping would be for JMS 1.1 for fqqn genreated depending on the consumer the client made in jms api.
   
   JmsQueueConsumer (createConsumer(Queue)-> shared durable
   JmsTopicConsumer (createConsumer(Topic)) -> non durable shared
   JmsTopicConsumer (createDurableconsumer(Topic)) -> non shared durable
   
   
   Def need tests for the above., and all the combinations.
   
   You could  look to make use of ParameterisedAddress (re use, like CompositeAddress is) like in Core to get extra queue config for a queue off the name, to allow for more configuration to be able to be present in the fqqn 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633289092


   Just to note and for the record, i am cautious of this change, as the FQQN was added for legacy stuff, making this change could be impacting to those expected behaviours. And building on it further encourages its use to proliferate. 
   
   Unlike stomp, which you bring up, AMQP has a very rich messaging structure that supports this, as such its not like comparing stomp or openwire which are limited, in this area.
   
   Personally i think for any higher level AMQP clients (e.g JMS/NMS) they would be better to expend time into adding support for the equivalent of jms shared sub in their abstractions, , so in case of NMS that would be to get that updated to  similar spec level with JMS 2.0
   
   I guess mapping would be for JMS 1.1 for fqqn genreated depending on the consumer the client made in jms api.
   
   JmsQueueConsumer (createConsumer(Queue)-> shared durable
   JmsTopicConsumer (createConsumer(Topic)) -> non durable shared
   JmsTopicConsumer (createDurableconsumer(Topic)) -> non shared durable
   
   For nonshared, non durable a user would just not use FQQN.
   
   Def need tests for the above., and all the combinations.
   
   You could  look to make use of ParameterisedAddress (re use, like CompositeAddress is) like in Core to get extra queue config for a queue off the name, to allow for more configuration to be able to be present in the fqqn 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
Havret edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633268105


   So the routing type is calculated there multiple times, using slightly different variations of the same logic. It's pretty messy tbh, maybe it would be a good idea to unify this, and extract one method to determine routing type. 
   
   The same story with fields `routingTypeToUse` and `multicast`. It is basically the same thing, isn't it? Or at least represent the same information. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633274657


   For anycast i wouldnt even bother, the anycast queue is expected in most cases to have been created when the queue address is, for JMS semantics anyhow.
   
   But thats just original logic added for FQQN, getMatchingQueue, so just change line 515 e.g. this
   
            if (!result.isExists()) {
               throw new ActiveMQAMQPNotFoundException("Queue: '" + queueName + "' does not exist");
   
   to return null, instead of throw exception (as like for non fqqn)
   
   do keep the rest of it.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633280730


   For AMQP clients they should really be using correct structures of AMQP not coding to FQQN. Its intent was solely for JMS legacy workaround where client has not yet upgraded to JMS 2 which supports shared subs etc. And in JMS 2.0 Amqp client like qpid they correctly set the amqp structures. This was focussed on jms usecases for non jms usecase in amqp, then should advocating using the correct amqp structures.
   
   E.g. in your case such where NMS doesnt yet support 2.0 (wip) then youre restricted to fqqn for shared subs. Obviously the whole issue is removed when you get NMS 2.0 upto 2.0 spec. 
   
   
   
   As such i would just assume if fqqn and queue not found it is a multicast 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633274657


   For anycast i wouldnt even bother, the anycast queue is expected to have been created when the queue address is, for JMS semantics anyhow. (E.g. the original remit of FQQN was allowing a JMS queue consumer bind to a mutlicat queue, to get around issues where a JMS client was still at 1.1 spec without shared sub support)
   
   re Topic (aka multicast)
   
   Thats just original logic added for FQQN, getMatchingQueue, so just change line 515 e.g. this
   
            if (!result.isExists()) {
               throw new ActiveMQAMQPNotFoundException("Queue: '" + queueName + "' does not exist");
   
   to return null, instead of throw exception (as like for non fqqn)
   
   do keep the rest of it.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633289092


   Just to note and for the record, i am cautious of this change, as the FQQN was added for legacy stuff, making this change could be impacting to those expected behaviours. And building on it further encourages its use to proliferate. 
   
   Unlike stomp, which you bring up, AMQP has a very rich messaging structure that supports this, as such its not like comparing stomp or openwire which are limited.
   
   Personally i think for any higher level AMQP clients (e.g JMS/NMS) they would be better to expend time into adding support for the equivalent of jms shared sub in their abstractions, , so in case of NMS that would be to get that updated to  similar spec level with JMS 2.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic edited a comment on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
clebertsuconic edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-719056558


   @Havret thanks for contributing.. I have seen other nice contributions from you...
   
   it's just that I hit another recent issue around mirroring and broker connection that I was working on, and I have seen another issue around this.. when I saw this I came with the -1... that's why I didn't bring this up before.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633293839


   I was editing my comment, as hadn;'t fully finished.
   
   Mappings i would expect from a black box testing:
   
   JmsQueueConsumer (createConsumer(Queue)-> shared durable (aka max consumer = -1, durable=true)
   JmsTopicConsumer (createConsumer(Topic)) -> non durable shared (max consumer = -1, durable=false, and tempoary)
   JmsTopicConsumer (createDurableconsumer(Topic)) -> non shared durable (max consumer = 1, durable = true)
   
   For nonshared, non durable a user would just not use FQQN.
   
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret commented on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN

Posted by GitBox <gi...@apache.org>.
Havret commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-628274586


   @gemmellr I'm bringing up JMS because JMS-AMQP mapping was the first thing you referred me to. Speaking in terms of AMQP spec, words like `shared`, `global`, `topic`, and `subscription` doesn't appear there even once, and you keep bringing them up. I'm not an expert in this area, I've never spoken with the authors of the spec. The only resources I have are spec itself, Artemis Source Code, and its documentation. Forgive me my ignorance but if I read in docs for amqp connector that
   
   > If the Terminus Durability is either UNSETTLED_STATE or CONFIGURATION then the queue will be made durable (similar to a JMS durable subscription) (...).
   
   And I see this being used to implement JMS features I interpret it this way.
   
   Shared Durable Subscription that you're recalling as the only feasible case to use TerminusDurability is not an AMQP feature. It's a way that JMS based clients can attach to multicast queues where topic maps to address and subscription name maps to durable queue name. The fact that the durable queue is created there is not an implementation detail from my perspective. When I configure broker I'm not configuring it in terms of topics and subscriptions. When I use FQQN I don't even need to know that such things exist. I have a broker with configured addresses and queues and I want to attach to them. In the ideal world, it would be great if it was possible to do it without the need to pre-configure (for instance to make development easier). 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633294661


   re:
   
   > So the behavior shouldn't change unless somebody relies on the fact, that the broker returns an error if one tries to attach to the queue that doesn't exist, instead of simply creating it.
   
   
   They do, if security settings etc, youll need tests 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633289092


   Just to note and for the record, i am cautious of this change, as the FQQN was added for legacy stuff, making this change could be impacting to those expected behaviours. And building on it further encourages its use to proliferate. 
   
   Unlike stomp, which you bring up, AMQP has a very rich messaging structure that supports this, as such its not like comparing stomp or openwire which are limited, in this area.
   
   Personally i think for any higher level AMQP clients (e.g JMS/NMS) they would be better to expend time into adding support for the equivalent of jms shared sub in their abstractions, , so in case of NMS that would be to get that updated to  similar spec level with JMS 2.0
   
   I guess mapping would be for JMS 1.1 for fqqn genreated depending on the consumer the client made in jms api.
   
   JmsQueueConsumer (createConsumer(Queue)-> shareddurable
   JmsTopicConsumer (createConsumer(Topic)) -> non durable
   JmsTopicConsumer (createDurableconsumer(Topic)) -> non shared durable
   
   I dont think its possible to map to autocreate a shared non durable, when presented fqqn with jms 1.1
   
   Def need tests for the above., and all the combinations.
   
   You could  look to make use of ParameterisedAddress (re use, like CompositeAddress is) like in Core to get extra queue config for a queue off the name, to allow for more configuration to be able to be present in the fqqn 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
Havret edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633278664


   So if I make these changes, it will work partially for multicast routing type (but all queues will be created using `createUnsharedDurableQueue` method. Not sure if this is desired. And it doesn't work at all for anycast, because of this if:
   
   ![image](https://user-images.githubusercontent.com/9103861/82762546-e6a5c180-9e01-11ea-9733-e56794a18a40.png)
   
   Any suggestions? 
   
   > For anycast i wouldnt even bother, the anycast queue is expected to have been created when the queue address is, for JMS semantics anyhow. (E.g. the original remit of FQQN was allowing a JMS queue consumer bind to a mutlicat queue, to get around issues where a JMS client was still at 1.1 spec without shared sub support )
   
   But we would end up with the implementation that's not in sync with what's available for instance for STOMP. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret edited a comment on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
Havret edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-718983078


   Yes, I totally agree (maybe besides the impact of auto-creation of queues to routing). I followed @jbertram advice and implemented topology management system similar to what is available for RabbitMQ. It is more robust and powerful solution. Sorry for the hassle. I will close this PR by tomorrow. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633293839


   I was editing my comment, as hadn;'t fully finished.
   
   Mappings i would expect from a black box testing:
   
   JmsQueueConsumer (createConsumer(Queue)-> shared durable
   JmsTopicConsumer (createConsumer(Topic)) -> non durable shared
   JmsTopicConsumer (createDurableconsumer(Topic)) -> non shared durable
   
   For nonshared, non durable a user would just not use FQQN.
   
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
Havret edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633272152


   I wish it was that simple. Unfortunately for anycast routing type we don't even get to this line. For multicast on the other hand,
   
   ```
   queue = getMatchingQueue(queueNameToUse, addressToUse, RoutingType.MULTICAST);
   ```
   in the original line 414 will throw an exception as the queue doesn't exist. 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN

Posted by GitBox <gi...@apache.org>.
Havret edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633233631


   @jbertram, @gemmellr 
   
   So after all it wasn't too productive discussion. I debugged the code, and apparently methods `createSharedDurableQueue` and `createUnsharedDurableQueue` don't explicitly create durable queues. :) The queues are durable by default if it's not specified otherwise. So if I drop "setDurable" bits, FQQN queues will be created as durable which makes sense, and doesn't break anything.
   
   However, there are still some things that need to be cleared up regarding my first question. 
   
   > There is a problem with the RoutingType mismatch. Previously it didn't matter what RoutingType receiver tries to attach to, as the implementation assumed pre-existence of the queue. Now the routing type matters, as we need to create a properly configured queue if it doesn't exist. That's why testQueueConsumerReceiveTopicUsingFQQN test fails currently. To make it pass I would need to subscribe to a topic instead of a queue. I'm not sure but is this a breaking change or not?
   
   STOMP implementation that @jbertram recently added https://github.com/apache/activemq-artemis/pull/3018 seems to silently ignore passed routing type when the mismatch occurs. Should I do the same here? It would be the safest way I guess, but what do you guys think? 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-634282836






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-719056558


   @Havret thanks for contributing.. I have seen other nice contributions from you...
   
   it's just that I hit another recent around mirroring and broker connection that I was working on, and I have seen another issue around this.. when I saw this I came with the -1... that's why I didn't bring this up before.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633280730


   For AMQP clients they should really be using correct structures of AMQP not coding to FQQN. Its intent was solely for JMS legacy workaround where client has not yet upgraded to JMS 2 which supports shared subs etc. And in JMS 2.0 Amqp client like qpid they correctly set the amqp structures. This was focussed on jms 1.1 legacy usecases as a bridging mechanism where client upgrade to 2.0 wasnt yet feasible. For non jms usecase in amqp, then should advocating using the correct amqp structures.
   
   E.g. in your case such where NMS doesnt yet support 2.0 (wip) then youre restricted to fqqn for shared subs. Obviously the whole issue is removed when we get NMS 2.0 upto 2.0 spec. 
   
   
   
   As such i would just assume in code that if fqqn and queue not found it is a multicast you want to create on
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
Havret edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633272152


   I wish it was that simple. Unfortunately for anycast routing type we don't even get to this line. For multicast on the other hand,
   
   ```
   queue = getMatchingQueue(queueNameToUse, addressToUse, RoutingType.MULTICAST);
   ```
   in the original line 414 will throw an exception as the queue doesn't exist. 
   
   There is quite a lot of code here, and a lot of non-obvious stuff is going on, that's why I wanted to break the flow and handle FQQN in separation to the old bits. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret commented on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
Havret commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633268105


   So the routing type is calculated there multiple times, using slightly different variations of the same logic. It's pretty messy tbh, maybe it would be a good idea to unify this, and extract one method to determine routing type. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633289092


   Just to note and for the record, i am cautious of this change, as the FQQN was added for legacy stuff, making this change could be impacting to those expected behaviours. 
   
   Unlike stomp AMQP has a very rich structure that supports this, as such its not like comparing stomp or openwire which are limited.
   
   Personally i think for any higher level AMQP clients (e.g JMS/NMS) they would be better to expend time into adding support for the equivalent of jms shared sub in their abstractions, , so in case of NMS that would be to get that updated to  similar spec level with JMS 2.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic closed pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
clebertsuconic closed pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret commented on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
Havret commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-718983078


   Yes, I totally agree. I followed @jbertram advice and implemented topology management system similar to what is available for RabbitMQ. It is more robust and powerful solution. Sorry for the hassle. I will close this PR by tomorrow. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce commented on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633267492


   If you look in  ProtonServerSenderContext, there already is a field derived and caclulated "routingTypeToUse"
   
   Also i would use the same logic  already in there thats working out if its a durable or non-durable queue (aka if its volatile)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633280730


   For AMQP clients they should really be using correct structures of AMQP not coding to FQQN. Its intent was solely for JMS legacy workaround where client has not yet upgraded to JMS 2 which supports shared subs etc. And in JMS 2.0 Amqp client like qpid they correctly set the amqp structures. This was focussed on jms usecases for non jms usecase in amqp, then should advocating using the correct amqp structures.
   
   E.g. in your case such where NMS doesnt yet support 2.0 (wip) then youre restricted to fqqn for shared subs. Obviously the whole issue is removed when you get NMS 2.0 upto 2.0 spec. 
   
   
   
   As such i would just assume if fqqn and queue not found it is a multicast you want to create on
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] Havret commented on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
Havret commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-634267603


   I'm not sure if mixing JMS concepts into this is the best idea. Determining whether a queue is durable/non-durable or shared/non-shared based on routing type seems to me pretty non-intuitive.
   
   Moreover, the way that JMS differentiate `createDurableconsumer(Topic)` and `createConsumer(Topic)` is based on `TerminusDurability` and `TerminusExpiryPolicy`. As @gemmellr  tried to explain to me throughout the big chunk of this thread, this property cannot be used to determine whether a queue is durable or not. 
   
   In the ideal world, I would just use `TerminusDurability` and `shared` capability. Having these two properties with FQQN, would make it possible to fully leverage Artemis Address Model in a dead-simple way:
   
   | Address   | Queue  | Routing Type  | Durable | Shared|amqp address  | capabilities  | durable |
   |---|---|---|---|---|---|---|---|
   | a1  | q1  | Multicast  | true  | true  | a1:q1 | shared, topic | 2|
   | a1  | q1  | Anycast| true  | true  | a1:q1 | shared, queue | 2|
   | a1  | q1  | Multicast  | true  | false| a1:q1 | topic | 2|
   | a1  | q1  | Anycast| true  | false  | a1:q1 | queue | 2|
   | a1  | q1  | Multicast  | false | true  | a1:q1 | shared, topic | 0 |
   | a1  | q1  | Anycast| true  | false  | a1:q1 | shared, queue | 0 |
   | a1  | q1  | Multicast  | false  | false| a1:q1 | topic | 0 |
   | a1  | q1  | Anycast| false  | false  | a1:q1 | queue | 0 |
   
   If we create a queue based on FQQN, only if the queue doesn't exist, we wouldn't break any existing users, as they are using FQQN to attach to pre-configured queues anyway. 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3124: ARTEMIS-2614 Create queues based on FQQN for AMQP protocol

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-719056847


   I will close it based on the discussion!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] michaelandrepearce edited a comment on pull request #3124: [WIP] ARTEMIS-2614 Create queues based on FQQN - AMQP

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #3124:
URL: https://github.com/apache/activemq-artemis/pull/3124#issuecomment-633274657


   For anycast i wouldnt even bother, the anycast queue is expected to have been created when the queue address is, for JMS semantics anyhow.
   
   re Topic (aka multicast)
   
   Thats just original logic added for FQQN, getMatchingQueue, so just change line 515 e.g. this
   
            if (!result.isExists()) {
               throw new ActiveMQAMQPNotFoundException("Queue: '" + queueName + "' does not exist");
   
   to return null, instead of throw exception (as like for non fqqn)
   
   do keep the rest of it.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org