yiisoft / yii2-queue

Yii2 Queue Extension. Supports DB, Redis, RabbitMQ, Beanstalk and Gearman
BSD 3-Clause "New" or "Revised" License
1.07k stars 295 forks source link

DB Driver: id and attempt properties not accessible during job execution #140

Open vercotux opened 7 years ago

vercotux commented 7 years ago

Problem

When a job is being executed all it gets is the $queue instance. It would be nice if the job could also access its own id, attempt number, and possibly other metadata during execute(). The metadata is there, it is simply not reachable from within a running job.

Use Case Examples

Workaround

A clumsy workaround for this is to attach an event handler to Queue::EVENT_BEFORE_EXEC which copies the $id and other relevant metadata from ExecEvent into the $job instance. Very inconvenient and not backwards-compatible with stock job interface, because we must ensure the job instances we use are able to receive these properties.

Ideas

  1. Store current $id, $attempt, etc. in Queue. (probably bad for any kind of parallelism)
  2. Add more parameters to call (eg. $event->job->execute($this, $extra);). Possible without modifying the job interface, but kind of hacky.
  3. Same as above, but also modify the job interface to predict a second parameter for execute(). (breaks BC)
zhuravljov commented 6 years ago

I would not want to overload the method with extra params about current job and break BC. I still believe that your rate case could be solved using a behavior with queue component as bridge between exec event and job execution:

class CurrentJobBehavior extends Behavior
{
    public $current;

    public function events()
    {
        return [
            Queue::EVENT_BEFORE_EXEC => function ($event) {
                $this->current = $event;
            }
        ];
    }
}
class SomeJob extends BaseObject implements JobInterface
{
    public function execute($queue)
    {
        echo $queue->current->id;
        echo $queue->current->attempt;
    }
}

This is more elegant solution to me.

rob006 commented 6 years ago

It is more like a hack, than a elegant solution. Basically you just using global variable to inject parameters to method execution - you will not be able to execute more than one job at once (for example executing different job in Job::execute()).

Also using queue-level behaviors to fulfill requirements of particular job is a nightmare in more complicated cases. If I have one queue which handles 10 types of jobs, and each job requires own behaviors/events, I need to mix them all at queue level and check type of job in behavior/event before doing anything. This affects performance and kills readability.

I think we really need something like beforeExecute(ExecEvent $event) and afterExecute(ExecEvent $event) in Job to attach job-level events and behaviors.

vercotux commented 6 years ago

@rob006's proposal would be the perfect solution for this (as long as the ExecEvent in afterExecute contains all the relevant information about whether the job was successful etc, which I assume it would).

@zhuravljov what you propose works but like Rob pointed out, only when you are sure there will be no parallel jobs.

zhuravljov commented 6 years ago

One worker executes one job at once (by design). Therefore job execution always related with before and after exec events. Each worker creates own queue component instance, that will contain its behavior instance. Proposed example will work correctly.

@rob006, yes, it is a hack. You are doing something wrong, if you forced to use it. I just suggest universal solution. But you can make ID property inside the job object and use it. It doesn't require using of individual behavior.

rob006 commented 6 years ago

One worker executes one job at once (by design).

And this is serious and annoying limitation of this library. I opened issue about this: #161. Also - are you sure that this always works in this way? What if I use sync driver and push another job inside of job execution?

You are doing something wrong, if you forced to use it.

What is wrong with using attempts number in job execution? Why Job cannot know its own ID during execution?

zhuravljov commented 6 years ago

And this is serious and annoying limitation of this library. I opened issue about this: #161.

I don't still have any ideas or proposals, how make simple and stable solution for your issue. This is broker limitation, no library. Each broker ensures to delivery of a message, and doesn't guarantee the order of messages, especially when you use delays and priorities. How are you going to group different messages to a batch on the transport level? It's impossible on a broker side. Second way to group jobs into one message as one batch job and send it. Then worker can divide it and execute separately. In this case, this is no limitation of the extension already, because you can solve the issue outside. But we as before cannot collect a batch with jobs from different process.

Also - are you sure that this always works in this way? What if I use sync driver and push another job inside of job execution?

Each native driver works in this way.

What is wrong with using attempts number in job execution? Why Job cannot know its own ID during execution?

It is coupling.

vercotux commented 6 years ago

It is coupling.

Yes, it is control coupling.

However, yii2-queue is already using coupling. Requiring all jobs to implement a specific interface is external coupling, which is tighter than control coupling! In other words, the jobs have to be specifically designed for this module... But they cannot know their own ID & attempt number?

This is like building a road that only cars which are designed specifically for that road can drive on. However the cars cannot know anything about the road they are driving on. They are built specifically for that road, why can't they know anything about it? You could say it is unnecessary, but as this issue demonstrates, sometimes it is necessary.

pptyasar commented 6 years ago

How to use CurrentJobBehavior?

developedsoftware commented 5 years ago

+1 for this

I am trying to access the same id that is returned when you push the job, from inside the job. Not possible at the moment I believe without doing some hack like in the above approach?

s1lver commented 1 year ago

Does this issue need to be resolved before a new version is released? Maybe it has lost relevance?