yii2-queue icon indicating copy to clipboard operation
yii2-queue copied to clipboard

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

Open vercotux opened this issue 6 years ago • 10 comments

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

  • Logging. If we want to do logging during execution we might want to know exactly which job ID each log record belongs to.
  • Conditional execution based on the attempt number. For example if we are interacting with an API service we might wish to increase certain request delays after a previously failed job execution attempt.

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)

vercotux avatar Sep 25 '17 15:09 vercotux

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.

zhuravljov avatar Nov 26 '17 04:11 zhuravljov

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.

rob006 avatar Nov 26 '17 13:11 rob006

@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.

vercotux avatar Nov 27 '17 05:11 vercotux

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.

zhuravljov avatar Nov 30 '17 06:11 zhuravljov

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?

rob006 avatar Nov 30 '17 10:11 rob006

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.

zhuravljov avatar Dec 04 '17 02:12 zhuravljov

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.

vercotux avatar Dec 04 '17 06:12 vercotux

How to use CurrentJobBehavior?

pptyasar avatar Apr 03 '18 11:04 pptyasar

+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?

developedsoftware avatar Oct 29 '19 18:10 developedsoftware

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

s1lver avatar Jan 23 '23 08:01 s1lver