Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The Binding for a Queue is on the Channel. Would it not be on a Subscribe Operation? #44

Closed
iancooper opened this issue Jan 14, 2021 · 19 comments
Labels
question Further information is requested

Comments

@iancooper
Copy link
Collaborator

Reason/Context

  • Why we need this improvement?
    When thinking about automation we should consider who might create something. In the amqp 0-9-1 model for primitives an exchange is a router, a queue is a subscription that router delivers to, and a binding links the two.
  • How will this change help?
    A publisher does not need to know about queues. The publisher sends to a topic but has no conception of who is subscribed to it. A subscriber cares about both the topic and the queue they subscribe to on the exchange.
    The common element is the exchange and the topics defined on that exchange - we might reason these are the channels. The queue provides guaranteed delivery to the client.
  • What is the motivation?
    Clarify that the queue defines how we subscribe to a topic, and is not part of the channel definition

Description

Please try answering few of those questions

  • What changes have to be introduced?
    We would need to move the queue definition to the bindings for the operation
  • Will this be a breaking change?
    Yes
  • How could it be implemented/designed?
    See above

Other

I am looking at SNS/SQS as well. Thinking about that model it makes more sense to define an SNS binding on the channel and an SQS binding on the operation, not SNS and SQS on the channel. For SNS there could be other protocols apart from SQS by which a consumer subscribes, so it may make more sense to bind those to the subscription for a given consumer, not the channel definition.

Any insight as to the why it was bound to the channel would be helpful.

BTW I am familiar with the idea from Hohpe ans a channel as a virtual pipe between producer and consumer, but I don't believe that means all of the bindings for it need to be defined in the AsyncAPI channel definition

@iancooper iancooper added the enhancement New feature or request label Jan 14, 2021
@derberg derberg added question Further information is requested and removed enhancement New feature or request labels Jan 14, 2021
@ericsampson
Copy link

Makes sense to me (hi Ian!)

From the RabbitMQ docs, "A binding is a relationship between an exchange and a queue". publishers are agnostic of queues.

Also a few other notes:

  • exchange-to-exchange bindings are supported, is this possible to represent?
  • queues can have x-queue-type property of "classic" or "quorum", I think this is currently missing.

@derberg
Copy link
Member

derberg commented Jan 18, 2021

@fmvilas do you remember why did you put it on the channel level initially?
@olvlvl any opinions from your side? I saw you added vhost to the queue. How do you consume it atm from the channel instead of the operation?

@olvlvl
Copy link
Contributor

olvlvl commented Jan 18, 2021

Hi everyone,

I appreciate that the "protocol bindings" are all grouped in this bindings property and not spread everywhere. Just because they share the same name we shouldn't confuse them with "RabbitMQ bindings" which are an implementation detail. As @iancooper said, a binding is between an exchange and a queue, with a routing key of course.

The following example demonstrates a channel we publish and subscribe to (This is for a PHP application, where the server and the consumer are split). BTW, we gave up on is because we do not find it useful.

  recipe.activated:
    publish:
      message:
        $ref: '#/components/messages/pubRecipeActivated'
    subscribe:
      message:
        $ref: '#/components/messages/subRecipeActivated'
    bindings:
      amqp:
        queue:
          name: recipes-service.events
          durable: true
          exclusive: true
          autoDelete: false
        exchange:
          name: recipes-service
          type: topic
          durable: true
          autoDelete: false

For a "queue-only" channel, we would use the following definition. Notice how we still define "exchange" to set up the binding, but we only define name (and vhost if needed) and nothing else because we are subscribing, we shouldn't define the exchange here.

  recipe.activated:
    subscribe:
      message:
        $ref: '#/components/messages/subRecipeActivated'
    bindings:
      amqp:
        queue:
          name: recipes-service.events
          durable: true
          exclusive: true
          autoDelete: false
        exchange:
          name: recipes-service

I hope this helps.

@fmvilas
Copy link
Member

fmvilas commented Jan 18, 2021

Yeah, as @olvlvl pointed out, AsyncAPI protocol bindings are not the same as RabbitMQ bindings. AsyncAPI bindings are defined at multiple levels (servers, channels, operations, and message) and they're a way to define protocol-specific information on AsyncAPI. The reason they're defined at Channel level is because a channel on AsyncAPI is the same as a topic/queue in RabbitMQ, and this information is concerning the topic/queue.

exchange-to-exchange bindings are supported, is this possible to represent?

We haven't gotten to that level of detail because that's not necessary to "consume" or use an API. In other words, this doesn't help you connect and receive and/or publish messages. That would make a great case for an extension.

queues can have x-queue-type property of "classic" or "quorum", I think this is currently missing.

Agree. Would you mind opening a different issue or PR for this?


Hope that helps!

@iancooper
Copy link
Collaborator Author

iancooper commented Jan 27, 2021

So we all agree that AMQP bindings and AsyncAPI bindings are not the same.

The question is where we define the properties of a queue that are required for its creation i.e. platform specific configuration. Should that be at the channel level or the operation level (publish or subscribe).

For Hohpe a channel is a virtual pipe but most commonly we tend to think about this as a logical address such as a topic or routing key. We publish to or subscribe from that topic to implement a 'channel'. There is a little bit of overlap between a channel type like pub-sub and a routing pattern like dynamic recipient list, so things do get a little hazy. But really the channel feels as though it should be independent of publishers and subscribers.

A queue is most commonly how we subscribe to a channel. Different consumers might have different queue definitions or might share one (competing consumers). But logically the queue is associated with subscribing to a channel - it provides store-and-forward for the subscriber. (Most middleware does not use a queue to publish to the channel, just TCP/IP, though we could make an argument that a point-to-point channel (such as ZeroMQ) both publishes and subscribes to a channel via a queue).

Although middleware has differences, we ought to be consistent about where the properties of a subscription (or a publication) are defined. It seems that if a queue is how we subscribe, then our definition should make that part of the operation not the channel. That would feel like the most common place to put subscription details.

Yes, I could choose to only add the queue to the channel when defining the AsyncAPI definition for an endpoint that has a consumer of that channel. But why is this logically preferable? Why does it make more sense for it to be associated with the channel, than the subscription to the channel?

If I had bindings which affected how I publish, say for example do I want publisher confirms on AMQP 0-9-1, does it make sense to have that as part of the channel, or as part of the publish operation binding? Surely that is a concern of how I publish, not what I publish to?

@ericsampson
Copy link

really the channel feels as though it should be independent of publishers and subscribers.

I agree. Both the publisher and consumer use (aka reference definition of) the channel, but neither one owns the definition of the channel. It is an independently defined/self-sufficient entity IMO. Cheers!

@olvlvl
Copy link
Contributor

olvlvl commented Jan 27, 2021

In the context of RabbitMQ, the "channel" is a routing key, which is a parameter, not a resource. Only exchanges and queues are resources, as such, that's all anybody can "own". I'm gonna use my previous example again, which is only a subscriber, to illustrate:

  recipe.activated:
    subscribe:
      message:
        $ref: '#/components/messages/subRecipeActivated'
    bindings:
      amqp:
        queue:
          name: recipes-service.events
          durable: true
          exclusive: true
          autoDelete: false
        exchange:
          name: recipes-service

My application owns the queue recipes-service.events which binds the exchange recipes-service with the routing key recipe.activated. Because my application owns that queue, I define the parameters to set it up. That's why we have durable, exclusive, and autoDelete defined there. When I have multiple bindings to the same queue I move the parameters of the queue to x-amqp-queues and use a $ref. My application doesn't own the exchange recipes-service, thus I'm only referencing the exchange by it's name.

Now consider the following example, which is a publisher:

  recipe.activated:
    publish:
      message:
        $ref: '#/components/messages/pubRecipeActivated'
    bindings:
      amqp:
        exchange:
          name: recipes-service
          type: topic
          durable: true
          autoDelete: false

This time my application owns the exchange, so I need to define the parameters to create that exchange. When multiple channels use the same exchange as use a $ref and move the configuration into x-amqp-exchanges. If my application didn't own the exchange I would only specify the name attribute.

Finally, we have a channel that's used to subscribe and publish:

  recipe.activated:
    publish:
      message:
        $ref: '#/components/messages/pubRecipeActivated'
    subscribe:
      message:
        $ref: '#/components/messages/subRecipeActivated'
    bindings:
      amqp:
        queue:
          name: recipes-service.events
          durable: true
          exclusive: true
          autoDelete: false
        exchange:
          name: recipes-service
          type: topic
          durable: true
          autoDelete: false

In that case, I defined all the things.

@iancooper
Copy link
Collaborator Author

But why would it not be

recipe.activated:
   bindings:
      amqp:
         exchange:
             name: recipes-service
             type: topic
             durable: true
             autoDelete: false
   subscribe:
      message:
        $ref: '#/components/messages/subRecipeActivated'
      bindings:
         amqp:
           queue:
              name: recipes-service.events
              durable: true
              exclusive: true
              autoDelete: false

If I look at the operation binding of Kafka, and we should be consistent about where we bind the same class of information, the definition of how we subscribe to the topic is an operation binding not a channel binding

channels:
  user-signedup:
    publish:
      bindings:
        kafka:
          groupId:
            type: string
            enum: ['myGroupId']
          clientId:
            type: string
            enum: ['myClientId']
          bindingVersion: '0.1.0'

It seems more sense to define the bindings for how we subscribe, as part of the operation binding, not part of the channel binding.

@olvlvl
Copy link
Contributor

olvlvl commented Jan 27, 2021

There's already a bunch of stuff on the operation level, that seems more related to the message itself rather than the "pipe". I never used any of these :)

channels:
  user/signup:
    publish:
      bindings:
        amqp:
          expiration: 100000
          userId: guest
          cc: ['user.logs']
          priority: 10
          deliveryMode: 2
          mandatory: false
          bcc: ['external.audit']
          replyTo: user.signedup
          timestamp: true
          ack: false
          bindingVersion: 0.1.0

@iancooper
Copy link
Collaborator Author

Yes, mostly message level properties, so they are at the wrong level as well. ReplyTo is a header value on a message really and might be better handled with traits I would have thought. ack and deliverymode really relate to how I consume, I'm not sure they really form part of a binding - I would suggest that is an implementation detail i.e. ack on consumer or manually ack once processed. I don't know why I would define those as part of a binding as they are imperative not declarative.

Is it worth submitting a PR to try and fix some of this?

@ericsampson
Copy link

@iancooper wrote:

But why would it not be (snip)

I 100% agree with Ian's proposal/example.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity 😴
It will be closed in 60 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation.
Thank you for your contributions ❤️

@ericsampson
Copy link

@iancooper did you end up creating a PR, or just gave up? :)

@iancooper
Copy link
Collaborator Author

@ericsampson I am going to create a new issue as I think I can explain it better.

@ericsampson
Copy link

@iancooper cool, feel free to tag me there

@iancooper
Copy link
Collaborator Author

@ericsampson Will do

@pcuriel
Copy link

pcuriel commented Jan 3, 2023

Hi everyone. Any news on this?

I'm facing an scenario where this change would be needed: supporting applications which bind more than one queue to the same routing key. If I'm not wrong, currently this is not supported, because we cannot declare more than one queue for the same channel.

Furthermore, with the new changes proposed for AsyncAPI 3.0 which introduce the Operations object, I think this change makes a lot of sense.

This way, following this proposal #44 (comment) by @iancooper we could have:

channels:
   recipeActivated:
      address: recipe.activated'
      messages:
         subRecipeActivated:
            $ref: '#/components/messages/subRecipeActivated'
      bindings:
         amqp:
            exchange:
               name: recipes-service
               type: topic
               durable: true
               autoDelete: false

operations:
   onRecipeActivatedUseCaseA:
      channel:
         $ref: '#/channels/recipeActivated'
      action: receive
      bindings:
         amqp:
           queue:
              name: use-case-a.recipes-service.events
              durable: true
              exclusive: true
              autoDelete: false

   onRecipeActivatedUseCaseB:
      channel:
         $ref: '#/channels/recipeActivated'
      action: receive
      bindings:
         amqp:
           queue:
              name: use-case-b.recipes-service.events
              durable: true
              exclusive: true
              autoDelete: false

@g-radam
Copy link

g-radam commented May 22, 2024

Just following up here. In issue #233 The 0.3.0 amqp binding documentation was changed to reflect the amqp binding schema. This change has prompted me to question weather the schema was correct in the first place - which has lead me to this issue.

The current amqp schema uses an "is: x" property to declare if a channel is an exchange OR a queue. This goes against what olvlvl suggests, which allows us to declare a new queue AND bind to an existing exchange. The current schema does not provide any mechanism to allow a queue to bind against an exchange to my knowledge? In my own AsyncAPI files, I've resorted to adding a x-exchange-name channel property when a channel represents an amqp queue so it knows which exchange to bind to.

I think if we were to allow both an exchange & queue to be defined within the channel binding this partially resolve the issue. However with that said, and as others have mentioned, Reusing exchange / queue information per channel and or moving or allow queue bindings on operations as @pcuriel has shown also are reasonable things to do in my opinion.

@derberg
Copy link
Member

derberg commented May 22, 2024

@g-radam @pcuriel @ericsampson @iancooper

hey folks, just wanted to encourage you to open a new issue and have a discussion in such open issue as knowledge from under closed issues will be lost and will not take us far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

7 participants