Skip to content

[Messenger] Standardized Options for Transport Processing #28324

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.

Sign up for GitHub

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

Jump to bottom
Closed
ragboyjr opened this issue Aug 31, 2018 · 9 comments
Closed

[Messenger] Standardized Options for Transport Processing #28324

ragboyjr opened this issue Aug 31, 2018 · 9 comments
Labels
Feature Messenger

Comments

@ragboyjr
Copy link
Contributor

ragboyjr commented Aug 31, 2018 •
edited
Loading

Description

Features like delaying a job before dispatching are things that would need to be handled per transport. I think it would be helpful to standardize the configuration around certain aspects of configuring transports/messages in the messenger component so that any transport can then optionally implement those options.

A few configuration options that could be worthwhile to put per message would be:

  • max tries - number of tries to restart the job, some jobs might be fine to restart 3 times, others should never be restarted in case of failure (think of like transactions with external services)
  • restart on failure - similar to max tries, might not need both options
  • delay till - don't queue the job until after the delayed amount
  • queue - run this message on a specific queue (might not be needed since you can route messages to different transports)

Now, each of those configuration items are handled in different parts of the system:

  • maxTries and restartOnFailure would be handled by a scheduler/worker
  • delayTill and queue would be specific to the transport to implement

The idea is that we could standardize the way we set these configuration options on the messages and then different transports could make sure to implement them instead of forcing each transport to define their style of configuration.

Example

$bus->dispatch((new Envelope($message))->with(new TransportConfiguration([
    'delay' => 3600,
    'queue' => 'something_else'
])));

Where TransportConfiguration looks something like:

class TransportConfiguration implements EnvelopeItemInterface
{
    private $config;
    public function __construct(array $config) {
        $this->config = $config;
    }

    public function getDelay(): ?int {
        return $this->config['delay'] ?? null;
    }

    public function getQueue(): ?string {
        return $this->config['queue'] ?? null;
    }

    public function getConfig() {
        return $this->config;
    }
}
@sroze
Copy link
Contributor

sroze commented Aug 31, 2018

Isn't it the same than #27215?

@ragboyjr
Copy link
Contributor Author

No, that's about adding the ability to retry (which is awesome), this would be standardizing transport config options that transports can respect (or not).

@sroze

@stof
Copy link
Member

stof commented Aug 31, 2018

But what would happen if you post a message asking for a 5s delay and your transport does not support delaying ?

for the queue proposal, this would be a bad idea, as it would couple the component to the AMQP semantic (other transport backends might not have a concept of queues). Use different transports instead.

@ragboyjr
Copy link
Contributor Author

I mentioned in the original description, the idea would be that it would be optional for transports to implement.

So, delaying is a somewhat common feature among queuing systems, SQS, Redis, DB, Rabbit MQ (with a plugin i believe) queues can all support it. I was hoping that each transport wouldn't have to define their own way of defining a delay.

So if you use SQS you need to use the SQS envelope item Configuration and options for using delay, but if you use a Redis transport, you'll need to whatever envelope item configuration they would provide and so on and so forth.

Does that make sense?

As far as queue, that's just a name that we could change, but I don't think it's specific to just amqp, but would apply to a redis transport, sqs transport, and then even a db transport.

@stof
Copy link
Member

stof commented Aug 31, 2018

As far as queue, that's just a name that we could change, but I don't think it's specific to just amqp, but would apply to a redis transport, sqs transport, and then even a db transport.

but that's something that the transport abstraction already covers. So creating a second concept in the component makes it more complex for no real benefit.

I mentioned in the original description, the idea would be that it would be optional for transports to implement.

We still need to specify what is expected to happen when you request a delay and the transport does not support it. If transports are free to either reject the message (saying they don't support it) or to dispatch it immediately, the abstration gets broken.

@ragboyjr
Copy link
Contributor Author

@stof and this is what makes symfony great :), you guys think about this stuff.

So, in the case of delay, one way to maybe support it is i think we'd need to add an empty interface for a transport to implement which can specify if it allows delays. And then a configuration option in the messenger system which allows us to configure the behavior to either throw if there is an unsupported transport option, or to silently let that through, and we default to the throw on exception.

And we could possibly use that system for any other standardized options we'd allow on the transport.

For queue, i don't full agree/disagree. Transports and queues are separate things.

We do have the ability to route to transports and could potentially make different instances of transports with a specific queue set which could work. If we are good with this type of implementation, I think it would be helpful if we added documentation on Creating your own transport on how to specify a queue when defining transport configuration (or something to that affect).

@stof
Copy link
Member

stof commented Aug 31, 2018

Note that in the case of AMQP, you don't even actually publish to a queue. You publish to an exchange (and then which queues this messages ends up in depends of how your broker is configured). So adding this concept of queues in Messenger might even make it more confusing.

@ragboyjr
Copy link
Contributor Author

understood, I don't have too much experience with AMQP, but ya, it seems like just routing messages to different transport instances which are configured properly would be the best option for that.

@sroze
Copy link
Contributor

sroze commented Sep 10, 2018

Agree with @stof. Let's close this one as there are no such "standardized options for transport processing". Even though things like retry would be great but there is already another issue for this.

@sroze sroze closed this as completed Sep 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature Messenger
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@stof @sroze @xabbuh @ragboyjr