Silverstripe-framework: RFC: A lightweight job interface that other modules can build on

Created on 17 Jan 2017  Âˇ  20Comments  Âˇ  Source: silverstripe/silverstripe-framework

Right now it's too hard for SilverStripe developers to write code that can be deferred to a worker queue. As a consequence, core code doesn't make as much use of this pattern as it could, and many developers only turn to such a pattern in the case of critical issues.

silverstripe/queuedjobs is a popular choice, but it's a relatively complex module with a lot of opinionated aspects, such as an admin UI and a number of DataObjects, that may be appropriate for some, but not all, deployments.

There are a number of other modules in the space, which indicates further fragmentation.

My proposal is that we have a set of core interfaces that are part of our default SS4 deployment and referenced heavily throughout core code. In addition, we refactor at least queuedjobs to have an adapter to these interfaces.

How would you interface with the JobRunner server?

The general idea would be that there's a job interface that we can use.

class WelcomeEmailJob implements Job
{
  use JobParams;

  function run(LoggerInterface $log) {
    $log->info("Sending email to" $this->params['Email']);
    $email = new WelcomeEmail();
    $email->setTo($this->params['Email']);
    $email->setData($this->params);
    return $email->send() ? Job::STATUS_SUCCESS : Job::STATUS_FAILURE;
  }
}

// ...

Injector::inst()->get('JobRunner')->queue(new WelcomeEmailJob([
  'Email' => Member::currentUser()->Email,
  'FirstName' => Member::currentUser()->FirstName
]);

The interfaces

This is my first cut at the interfaces. I expect that it will need a lot of input from the community, but it covers the basic.

interface JobRunner
{
    /**
     * Queues a job for execution when the server is next available to.
     * @return JobStatus
     */
    function queue(Job $job, $queuename = null);

    /**
     * Get the status of a running job
     * @return int A Job::STATUS constant
     */
    function status(JobIdentifier $identifier)
}

interface Job extends Serializable
{

    const STATUS_QUEUED = 1;
    const STATUS_RUNNING = 2;
    const STATUS_SUCCESS = 3;
    const STATUS_FAILURE = 4;

    /**
     * Run this job immediately.
     *
     * @return int Job::STATUS_SUCCESS or Job::STATUS_FAILURE
     */
    function run(LoggerInterface $output, JobRunner $runner);
}

/**
 * A marker object from which statuses can be retrieved
 */
interface JobIdentifier extends Serializable
{
    /**
     * Return int A Job::STATUS constant
     */
    function getStatus();
}

/**
 * Trait to help with implementing serializable
 */
trait JobParams
{
  protected $params;

  function __construct($params) {
    $this->params = $params;
  }

  function unserialize($serial) {
    $this->params = unserialize($serial);
  }

  function serialize() {
    return serialize($this->params);
  }
}

Default implementation

The core interfaces can come with an execute-immediately implementation. Anything more fancy will require an additional module installed. The idea is to let people write code against the JobRunner / Job interfaces without needing to bring in any job scheduling functionality that's not appropriate in the current case.

class ImmediateJobRunner implements JobRunner
{
      private $isRunning = false;
      private $queue = [];

    function run(Job $job)
    {
                $log = Injector::inst()->get('Logger');
                $jobStatus = new ImmediateJobStatus();
                $this->queue[] = [ $job, $jobStatus ];
                if(!$this->isRunning) $this->runQueue();
                return $jobStatus;
    }

        private function runQueue()
        {
              $this->isRunning = true;

               while(count($this->queue) > 0) {
                       [ $nextJob, $status ] = array_shift($this->queue);
                       $status->setStatus(Job::STATUS_RUNNING);
                       $status->setStatus($nextJob->run(Injector::inst()->get('Logger'), $this));
               }

               $this->isRunning = false;
         }
}

class ImmediateJobStatus implements JobStatus
{
    private $status;

    function __construct($status = Job::STATUS_QUEUED) {
        $this->status = $status;
    }

    function serialize($status) {
        return serialize($this->status);
    }

    function unserialize($serial) {
        $this->status = unserialize($serial);
    }

    function getStatus() {
        return $this->status;
    }

    function setStatus($status) {
        $this->status = $status;
    }
}

Advanced features

Scheduled jobs

In the default implementation, it's not expected that project / module developers provide specific times for when tasks should execute, instead grouping tasks into logical bucket such as 'regular', 'daily', 'weekly', and 'monthly'. This could be provided a number of service definitions in the named keys of JobRunner.recurring_jobs of config.

JobRunner:
  recurring_jobs:
    regular:
      - class: DoSomethingJob
         constructor: [ "MyArg", "OtherArg" ]

In the core implementation, there would be a simple CLI script for executing a named set of recurring jobs. A sake-compatible URL, /dev/run-recurring-jobs/<groupname>, would be a simple solution. So a developer could set up a cron such as this:

0 * * * www-data /var/www/framework/sake dev/run-recurring-jobs/regular
15 09 * * * www-data /var/www/framework/sake dev/run-recurring-jobs/daily
30 09 * * 1 www-data /var/www/framework/sake dev/run-recurring-jobs/weekly
45 09 01 * 1 www-data /var/www/framework/sake dev/run-recurring-jobs/monthly

The point is that the sysadmin is better placed to decide exactly when tasks should run, and whether "regular" tasks should be hourly or every 5 minutes.

More sophisticated modules could provide better tools for organising job scheduling, on a bucket-by-bucket or job-by-job basis.

I'd appreciate more feedback on how granular the in-module mechanism for specifying recurring tasks should be.

Build tasks

Build tasks are jobs that can be triggered manually by a developer. In SS3, everything that implements BuildTask is treated this way, by constructing a singleton.

Under this model, build tasks could be registered in in the yaml config e.g. in JobRunner.manual_jobs:

JobRunner:
  manual_jobs:
      - class: CleanupTestDatabasesTask
         title: "Clean-up test databases"
      - RebuildDatabaseTask
         title: "Update database schema (a.k.a. dev/build)"

The existing build task API would be deprecated; we might build an adaptor to help with the transition. In any case, build task is excessively coupled to HTTPRequest and to direct output, and is in need of a refactor.

Job prioritisation

By default there will be no job prioritisation. However, because there is a $queuename argument to JobRunner::queue(), different job modules could prioritise some queues above others. It might make sense to recommend to developers that they write high and low priority jobs into queues named "high" and "low". For more granular control developers can use other names.

Compound jobs

As long as jobs can queue other jobs, compound jobs are reasonably straightforward, as shown below. This automatically infers an order in the jobs, which is that RegenerateStaticPagesJob is called first, and then RegenerateStaticUrlJob are called in any order.

class RegenerateStaticPagesJob {
  function run(LoggerInterface $log, JobRunner $runner) {
    $urls = (new Page())->getAllUrlsToCache();
    foreach($urls as $url) {
      $runner->queue(new RegenerateStaticUrlJob([ 'url' => $url ]));
  }
}

class RegenerateStaticUrlJob {
  use JobParams;

  function run(LoggerInterface $log, JobRunner $runner) {
    Director::test($this->params['url']);
    // ...
  }
}

If you could support a CallableJob class, serializing a closure, this code could be made even leaner.

Things that can be done as add-on APIs

Different job runners per job class, etc

It might be useful to provide different backends for different kinds of jobs. This could be implemented by providing a dispatcher implementation of JobRunner, and so we don't need change the core API.

Composite jobs with different routing logic

For very large jobs it can be helpful to split jobs into small pieces. These jobs might be able to be executed in any order (e.g. splitting across multiple back-ends) or they might be order dependent.

A job runner needs to decide whether it is critical to run jobs in order or not. If we say "always assume jobs run in order" that limits the design of our runners. If we say "always assume order doesn't matter" then some code might break.

It's possible that we could have some mechanism in the queue() calls to define a partial order, but we wouldn't want to bloat the API. Perhaps an array of JobStatus objects, $mustRunAfter, could be optionally provided. In the case of a single-thread executor, this argument could be ignored, meaning that simple implementations are still possible.

Alternatively it might be possible to define a job Decorator that performs this kind of check within the current API, by checking whether the other jobs have been executed, and if not re-queueing the job.

In the spirit of keeping the core API lean, that might be better, but it would mean that:

  • any module that wishes to implement order-dependent jobs is going to have to have another dependency. We could alway provide the JobDecorator as part of the core package, while keeping it out of the JobRunner's domain.
  • job deferral will have to happen by starting a job, running the check, and if the preconditions aren't met, requeueing the job. I don't know if there are more optimal implementations that would be important to allow support for?

Jobs from Callables / promise-style "then()" method

A CallableJob class could provide a nice interface for interacting with callables. It might look something like this. Serializing the callables left as an exercise for the reader. This could be an optional add-on module that needn't be a core dependency – it should be able to operate as a layer on top of the core job system, meaning that it can be plugged into any compatible JobRunner.

class CallableJob {
  function __construct(callable $call) {
    $this->call = $call;
  }

  function run(LoggerInterface $log, JobRunner $runner) {
    $call = $this->call;
    return $call($runner, $log);
  }

  /**
   * @return CallableJob a job with a subsequent job attached
   */
  function then($nextCall) {
    $call = $this->call;
    return new CallableJob(function($log, $runner) {
      $result = $call;
      if($result) {
        $runner->queue(new CallableJob($nextCall));
      }
      return $result;
    });
  }
}

Features not included

Retry / re-queue support

Sometimes it's useful to configure a certain number of retries for a given job. It would be possible for a JobRunner implementation to let a project developer configure retry rules on a job-class by job-class or job-queue by job-queue basis, however, that doesn't let module developers provide any call-specific guidance as to how retries should be handled.

In order to make the Job decorator described in the previous section, we'd need an ability to requeue. Requeue could also be used to implement project-specific retry rules.

A requeue could be handled by passing the job back to JobRunner::queue() but then any callers that were relying on the original JobStatus object would have no indication that the job was still going to be executed. Having JobRunner::requeue(JobStatus $status) would let a runner maintain the same status meta-data when putting the job back onto the stack.

If a job were able to modify its parameter data before requeuing, this would let developers implement things such as a "retriesRemaining" counter. However, this would require a job to requeue as part of the job execution itself, which might require an API based on something other than the jobstatus object (unless this was also passed to Job::run()).

BuildTask configuration

A Job and a BuildTask cover a lot of the same ground and so I've recommended that 'manually executed Jobs' replace the build task.

The main thing that we don't currently have a good understanding of is user-supplied parameters. Rather than an chucking anything in the constructor, the job would need have a more strict set of parameters, and perhaps a getParamMetadata() method to assist with reflection. This might end up as something like this:

class SendWelcomeEmailJob implements Job {
  function getParamMetadata() {
    return [
     "FirstName" => "First name",
     "Surname" => "Surname",
     "Email" => "Email address",
    ];
  }

  function run($params, LoggerInterface $output) {
    $email = new WelcomeEmail();
    $email->setTo($params["Email"]);
    $email->setData($params);
    if($email->send()) {
      return Job::STATUS_SUCCESS;
    } else {
      $output->error("Failed: " implode(", ", $email->getFailures()));
      return Job::STATUS_FAILURE;
    }
  }
}

But passing $params to run isn't well supported by the current JobRunner::queue() method, which expect parameters to be loaded in before it receives a job object.

One way of solving that would be to amend queue so that it takes a pair of arguments, classname & parameters:

Injector::inst()->get('JobRunner')->queue('WelcomeEmailJob', [
  'Email' => Member::currentUser()->Email,
  'FirstName' => Member::currentUser()->FirstName
]);

Another approach would be to distinguish an empty job template from a version of that job with parameters loaded:

$jobTemplate = new WelcomeEmailJob;
$job = $jobTemplate->withParams([
  'Email' => Member::currentUser()->Email,
  'FirstName' => Member::currentUser()->FirstName
]);

Both of these approach lend themselves more to a base class (or interface + trait pair) than they do to a flat interface, as there would be some boilerplate for parameter management.

These approaches might make it also harder to make an object such as CallableJob, which has a callable/closure passed into it constructor – something that is clearly inappropriate as a build task, but very helpful for writing more lightweight job-based code (although serializing closures might present challenges). It may seem like an edge-case but we risk diluting the power of jobs to only those things that could conceivably be build tasks.

Delays

There's no way of doing a delayed job. However, in principle a specific implementation could provide a DelayedJob job decorator that somehow worked in concert with an appropriately designed jobrunner backend.

Alternatives

  • Make QueuedJobs leaner: We could split out things like support for scheduling and the admin UI into optional packages, and have the core API be much leaner, and incorporate it as a core dependency. However, I think that this would probably be more work than simply building an adapter to these new interfaces in QueuedJobs.
  • Have "if QueuedJobs is installed" checks in core code: I don't see branching logic like this as desirable, compared to tying to a lightweight dependency.
  • Use PHP's built-in serialisation rather than hydratable: This might be a good idea. I was a bit nervous about serialising things like large sets of data objects in favour of re-fetching such data from the database, and having an explicit hydratable interface make this more obvious to the job developer, but the JobParams trait kind of undoes this, and maybe I shouldn't reinvent the wheel?
  • Use closures instead of job objects: You could create CallableJob that receives the closure in its constructor.
  • Use BuildTask as the starting point: It's such a lightweight interface and most of what is there already is wrong; I think it's better to start from scratch.
affectv4 changminor efforhard impachigh rfaccepted typenhancement
Source
picture sminnee
👍4

All 20 comments

Unsure whether JobStatus should extend Hydratable. Their purpose is very different.

assertchris picture assertchris on 17 Jan 2017

Shouldn't JobRunner::status() should take a Job as the first parameter?

If you pass the same job to JobRunner::queue() twice, it's going to get executed with the same parameters twice. However, each will return a separate JobStatus object. JobStatus is the class that represents a single execution of a job, and so it's what you should pass to JobRunner::status().

Arguably, it might be better to recommend that users simply call JobStatus::getStatus() rather than pass the object back to the job runner, and delete the JobRunner::status() method.

Unsure whether JobStatus should extend Hydratable. Their purpose is very different.

Let's say you're putting jobs onto a resque queue and then running them with a separate worker. Let's imagine that we've done a submission that triggers a "send 1000 emails" job and then we want to do an ajax poll to keep up-to-date on the job status.

So the initial submission is going to need to save a reference to the JobStatus somewhere and then retrieve it for the PHP request responding to the ajax poll.

My thinking was that the JobStatus could be dehydrated in the initial submission and rehydrated elsewhere, the dehydrated job status could be provided as POST data in that subsequent request.

Some pseudocode:

Initial submission:

function initialSubmission($data) {
  $jobStatus = new SendLotsOfEmailsJob($data);
  $payload = json_encode([
    'job-status-class' => get_class($jobStatus),
    'job-status' => $jobStatus->toArray(),
  ]);
  return new HTTPResponse($payload);
}

Ajax handler on the client:

$.post('/initialSubmission', someData, function (jobStatusJsonResponse) {
  $('status').text("Starting emails");
  setInterval(function() {
      $.submit('/ajaxPoll', jobStatusJsonResponse, function (pollJsonResponse) {
        $('status').text(pollJsonResponse.message);
      }
  }, 1000);
});

The Ajax poll handler:

function ajaxPoll($jsonPayload) {
  $jobStatusClass = $jsonPayload['job-status-class'];
  $jobStatus =   $jobStatusClass::fromArray($jsonPayload['job-status']);  

  $messageMap = [
    Job::STATUS_QUEUED => "Waiting to start",
    Job::STATUS_STARTED => "Sending emails",
    Job::STATUS_SUCCESS => "All emails sent",
    Job::STATUS_FAILURE => "Sorry, there was a problem",
  ];
  $message = $messageMap[$jobStatus->getStatus()];
  $payload = json_encode([ 'message' => $message ]);
  return new HTTPResponse($payload);
}

Now having written this there's probably more boilerplate than is needed – particularly the need to pass both the job status class and its data as separate items – but the key thing was that I needed a way of serialising a job status object and unserialising it in a subsequent request. Regarding serialising, turning into JSON via a PHP array is my preference.

This could be good too: http://php.net/manual/en/class.jsonserializable.php

sminnee picture sminnee on 17 Jan 2017

If the JobStatus implemented JsonSerializable, and there was a separate JobHydrator class, then you could do something like this:

Initial submission:

function initialSubmission($data) {
  $jobStatus = new SendLotsOfEmailsJob($data);
  $payload = json_encode(['status' => $jobStatus]);
  return new HTTPResponse($payload);
}

Ajax handler on the client:

$.post('/initialSubmission', someData, function (submissionResponse) {
  $('status').text("Starting emails");
  setInterval(function() {
      $.submit('/ajaxPoll', submissionResponse.status, function (pollResponse) {
        $('status').text(pollResponse.message);
      }
  }, 1000);
});

The Ajax poll handler:

function ajaxPoll() {
  $jobStatus =  JobHydrator::fromJSON($payload);

  $messageMap = [
    Job::STATUS_QUEUED => "Waiting to start",
    Job::STATUS_STARTED => "Sending emails",
    Job::STATUS_SUCCESS => "All emails sent",
    Job::STATUS_FAILURE => "Sorry, there was a problem",
  ];
  $message = $messageMap[$jobStatus->getStatus()];
  $payload = json_encode([ 'message' => $message ]);
  return new HTTPResponse($payload);
}
sminnee picture sminnee on 17 Jan 2017

In principle, I support the notion of having a standard API for queuing and job status checking. The exact API isn't something I have to many opinions about at this stage, but potentially Laravels API could be an inspiration: https://laravel.com/docs/5.3/queues

However, I have some thoughts and notes on the missing features that I think could drastically change the API if we don't hash them out in the beginning.

Scheduled tasks

The trick is that scheduling could be possible / not possible depending on the backend. Some backends might have support for it; some might need to rely on SilverStripe triggering it via “cron” functionality.

If we are looking at workers that are using SilverStripe, they could share the same database so when to execute could be stored there. If the job is more of a message queue implementation that doesn’t have access to the Queuers database, the job would need to be implemented with a custom build task / cron task / other.

Job priorities and/or different queues

Having different queues with different priorities and/or jobs with different priorities could potentially be useful. The email queue might be a lower priority than the embargo / publish queue. However, I haven’t had any real use cases where this has been a priority. Hopefully, someone might have some experience with this.

Different job runners per job class, etc

One job might use e.g. AWS SQS, and another would use Redis?


As you said, it might be useful, but I’m not sure how helpful and if it causes more pain and bugs than it solves.

Compound jobs and job ordering controls

Job flow control is tricky and but also can be very useful. For comparison, I’ve recently looked into the AWS Simple Workflow Service (which as follows the “if it’s called simple, it’s probably not” standard) and is somewhat of a beast.

They use a model where you have a Scheduler, a Worker and a Decider. The scheduler queues a job; the worker does the task and the Decider queues, finish or queues another job. So the business/ordering logic is contained in the Decider part. The decider would have logic for re-queueing, branching, etc. I believe it’s some enterprise strategy pattern.

This would add another interface, but have the benefit of keeping the workers and Scheduler simpler. 


Retry / re-queue support

The MQ implementations I’ve seen/used configures this on the queue itself (or global default) and if it cannot be complete, sends it to a ‘dead letter’ queue.

The message broker decides and manages this. The data on how many retries that have been done could potentially be stored in the JobStatus.

I have a feeling that allowing the workers to modify the job params data could cause subtle bugs. 

Maybe 
1) JobRunner executes job
2) checks status, if failed
3) check if re-queuing is enabled and then queues the job again and updates the job status counter?

Sorry, if I'm a bit unclear and not 100% entirely coherent :)

stojg picture stojg on 18 Jan 2017
👍1

/cc @nyeholt @tractorcow @chillu would be good to get your input.

sminnee picture sminnee on 18 Jan 2017

I was intending to replace the custom worker system in silverstripe/staticpublishqueue with silverstripe/queuedjobs in the upcoming SS4 release, but would 100% prefer a standard Job interface that would allow people to trigger the job however best suits their needs.

For example, due to the way staticpublishqueue works (curl-ing the page) there's no reason the (very resource intensive) cache regen even needs to happen on the web box. With a generic enough Job interface someone could set up SQS + Lambda functions to do that part.

Will hold off on the queue refactor while we see where this RFC goes 😸

edlinklater picture edlinklater on 18 Jan 2017

Definitely have some comments to add, once I have some time later this week :)

nyeholt picture nyeholt on 18 Jan 2017

Thanks @nyeholt, that would be appreciated. I guess there are two questions it would be good to get your input on:

  • Unifying standards by creating a new one (https://imgs.xkcd.com/comics/standards.png)
  • The specifics of the API design

On the first point; I'd be open to the idea of drastically trimming down queuedjobs and splitting everything else out into a separate package (or several packages). In particular, I'd expect that the admin UI, the default tasks, and the most of the job processors wouldn't be included. I just wasn't sure whether that was actually the best solution.

Looking at the QueuedJob interface definition specifically there are some aspects that I'm not sure are necessary for a lean & mean interface that potentially becomes a core dependency:

  • The meta-data handled by setJobData() / getJobData(), and the implication that a job might be a multi-step job that's facilitated by calling setup() once and process() multiple times. In my sketch above I've suggested that this kind of use-case could be handled with a job that generates more jobs. This would let developers handle large/complex jobs without needing a special API for it. It would require a requeue() method of some kind as well as mutable state attached to the job.
  • The distinction between immediate, queued, and large jobs in getJobType(). I've suggested a "named queues" feature that could be used to facilitate more complex actions without needing specific APIs for them. My goal was to keep the core API simple while allowing for the possibility of sophisticated set-up in complex deployments.

At this point, it's not obvious to me whether a new module or a change to QueuedJob is better. But, like I say, keen to get your views.

Another way of looking it would be to re-brand QueuedJob as a "multi-step job". I could see it interacting with the underlying job system something like this:

abstract class MultiStepJob implements Job {
  abstract function isFinished(); 
  abstract function start($runner, $log); 
  abstract function step($runner, $log); 

  function run($runner, $log) {
    if($this->step == 0) $this->start($runner, $log);
    $this->step($runner, $log);
    if(!$this->isFinished()) {
      $this->step++;
      $runner->requeue($this);
    }
}

Note that the class-rename helper in silverstripe-upgrader would limit the upgrade cost of choosing new classnames as part of the refactor.

sminnee picture sminnee on 18 Jan 2017

If we were to proceed with this, it would make sense for the dev/build code to be refactored to a job for interface consistency.

sminnee picture sminnee on 23 Jan 2017
👍1

I'd like to suggest having some kind of post-request-queue as standard to compliment the immediate queue job.

This would be a queue that executes once the response has been sent to the browser.

dhensby picture dhensby on 23 Jan 2017
👍1

+1 to post-request-queue. When first starting out, this is the simplest method to get this kind of functionality rolling and it's ideal when you aren't given much time to complete a task (think small business, agency work)

silbinarywolf picture silbinarywolf on 23 Jan 2017

Yeah, that makes sense to include in whatever the minimal recipe is, as it doesn't require extra config.

sminnee picture sminnee on 23 Jan 2017

A wall of text to come, but there's a summary up the top. For the most part, I think the simpler the API the better; the complexity of something like QueuedJobs should be able to be wrapped up underneath that API surface.

TL;DR

interface JobRunner {
    /**
     * @param mixed $c
     *      The thing to be added to an execution queue; a closure, a string for injector lookup, or Job instance
     * @param array $args
     *      Arguments to be provided to the executed item. 
     * 
     * @return JobIdentifier
     */
    public function queue($c, $args = null);

    /**
     * Called by the framework when it considers it a suitable to run the jobs. 
     * 
     * Note that third party modules could use this call as the point at which it performs
     * serialization and DB storage, or upstream queue engine sending
     */
    public function executeJobs();

    /**
     * Get the status of a running job
     * 
     * @return JobStatus
     */
    public function status(JobIdentifier $jobId)
}

// agreed on the job interface
interface Job extends Serializable {
    /** 
     * Not sure yet whether run should always take in particular framework determined
     * parameters or not. I'm leaning towards "probably", mainly due to consistency when supporting
     * closures
     */
    public function run(Logger $logger);

}


interface JobStatus {
    public function getStatus();
}

interface JobIdentifier {
    /** Retreives an identifier that can be used to retrieve job status later */
    public function getIdentifier();
}


// a job that is 'managed' in its execution by a job runner that cares enough to do so
interface ManagedJob {
    public function getMaxRetries();
    public function isComplete();
    public function getCurrentStep();
    public function getTotalSteps();
}

// a job that should be scheduled; a separate queue runner that handles these would be needed
interface ScheduledJob {
    public function nextRunTime();
}

The main 'differences' here being;

  • queue() should be able to handle a callable (namely, a closure, but possbily an anonymous class in PHP7)
  • queue() should return an identifier that can be used to look up the status later if the developer cares about it
  • Added JobIdentifier interface
  • Interfaces relating to ManagedJob and ScheduledJob; these would be used by queue runner implementations that cared about them

The separation of JobStatus and JobIdentifier is that the lookup for the current status of a job doesn't really make sense to be tied to an object that implements a method called 'getStatus'...

I think it's important that the JobRunner interface does _not_ use "run" as its method name, as it needs to be clear to a developer that whatever they pass will _not_ execute "now" or before the end of their current method. Another name than "queue" may be nicer, but the name must be clear that the functionality is not executing "now".

Anyway - some background discussion and mental dumping to get to the above points; I've approached this by first looking at whether meeting the simplest of use-cases can be scaled up to meet the more complex ones. From the discussion already in the RFC, I think that's doable, and there's probably some peripheral things that can be accounted for too. Some of what's below is repeating things people already know, but given the RFC doesn't really touch on the 'whys' at present, I figured the background is useful for others reading along that haven't needed to deal with the problem.

For me, the motivations behind the queuedjobs module were (not necessarily in prioritised order, but somewhat indicative of importance)

  • Don't execute on the running request thread (or at least, before a user gets their response)
  • Don't execute lots of things at once and swamp a server's resources (either DB / web server); ie, queued.
  • Allow some prioritisation of things; some things will take a long time to get through, others not so much
  • Provide some level of 'health' monitoring; memory usage, catching jobs that have stalled / excepted / are infinitely looping; and recovering from those failures.
  • Allow things to be executed in the future
  • Allow things to be executed on a different server than the one where the job was created
  • Execute in a security context of the user that created the job in the first place
  • Provide some feedback to system users of what's going on
  • Allow the creation / triggering of jobs to be done from the UI, rather than just requiring code or CLI interaction
  • Allow execution to be re-entrant; allow some level of failure or resumption of things if needbe.

Many of these are still valid, I think the key though is that they don't _all_ need to be accomodated by the base implementation, or in some cases can be accomodated by concrete implementations underneath a much simpler API surface. Which is to say; a more complex JobRunner should be able to handle the simplest of Job implementors, and the simplest JobRunner should still be able to handle a more complex Job implementation - at least, to have its functionality executed.

So, the simplest case; let's say I have code that looks like

public function someaction () {
    $param = $this->request->getVar('something');
    $items = MyObject::get();

    foreach ($items as $item) {
        $item->executeBehaviour($param);
    }
}

If there are several hundred (n) $items, that code will make the request block until executeBehaviour has executed _n_ times - an obvious problem.

So, the framework should provide a mechanism that allows a developer to say "run this code in a way that won't block my user getting a response". 

class MyController {
    /** @var JobRunner */
    public $runner;


    public function someaction () {
        $param = $this->request->getVar('something');
        $myfunction = function () use ($param) { 
            // as above 

        };
        $this->runner->queue($myfunction);
    }

}

interface JobRunner {
    // allow arguments
    public function queue($c, $args = null);
}

// Naive impl
class SimpleRunner implements JobRunner {


    public function queue($c) {
        if (is_callable($c)) {
            call_user_func_array($c, ... tbd ...);
        } else if ($c instanceof Job) {
            $c->run(...);
        }   
    }
}

// forking impl
class ForkedRunner implements JobRunner {

    public function queue($c) {
        pcntl_fork();
        if (is_callable($c)) {
            $c = new CallableJob($c); 
        } 

        $c->run(...);
    }
}

// post response impl
class PostResponseRunner implements JobRunner {
    private $jobs = [];

    public function queue($c) {
        $this->jobs[] = [
            'callable'  => $c // or, new CallableJob($c) for closures ,
            'args'      => ... tbd ...,
        ];
    }


    // called by a post-request filter or similar
    public function executeJobs() {
        foreach ($this->jobs as $j) {
            $j->run(...);
        }
    }

}

This immediately implies that the "Job", or callable functionality, should be separated from the metadata/state of that job, and that those job details are purely optional. Which the RFC addresses already - though I do think that this is potentially one of the more common use-cases for which a job could be used, and may be worth supporting directly instead of requiring it be wrapped in a CallableJob (trivial as that may be - it's probably better that the queueing mechanism handles it) - and that while there are some libraries that can serialize closures, it should not be expected.

So - tl;dr point 1: The simplest use-case of the API should be to support callables.

From this point onwards, I think almost everything could be handled by non-core concrete implementations, but with the specific interfaces defined as part of core. It may be simplest to provide a core implementation, but that's not really what the RFC has to answer immediately :). So everything below this line is more discussion and considerations, to see where the proposed API fails to handle a use-case.

The next more complex use-case - where there is state expected to be tracked, either because the timespan of the job is expected to be "a while" or there is a desire to provide some feedback to users, either real-time or post-execution.

One of the most common complaints or confusing aspects of the current queuedjobs implementation is that it expects the developer to be aware of the serialize-ability of its stored data, as well as things like the job signature and other "state" things related to how it exists in the queue environment. The problem is that the constructor is called in two separate contexts; either by the developer when creating the job, _or_ by the job worker when executing the job class; there's possibly some merit in using ReflectionClass::newInstanceWithoutConstructor, it is a little bit of "magic" that is possibly even more problematic or confusing for developers.

If it is considered more confusing than not, I see the simplest way to go is to accept the name of the job class as a parameter, which will be looked up from the injector, and the "data" to be set into the job as an arguments array (or JobData object?). In which case, a Job class should have a method through which parameter data will be passed; using a string identifier for the job also makes it somewhat simpler for jobs to be overwritten from an injection standpoint. 

public function queue($c, $args = null) {
    if (is_string($c)) {
        $job = Injector::inst()->create($c);

    }

    $this->jobs[] = ['callable' => $job, 'args' => $args];
}


public function executeJobs() {
    foreach ($this->jobs as $j) {
        $j->populate($j['args']);
        $j->run(...);
    }
}

Of course, accepting an actual Job instance would need to be supported too; it would be the responsibility of the developer to ensure the constructor behaved in both contexts.

Additionally, I think the separation of job state (as far as any external management module cares about it) should be stored separately from job data / parameters. Effectively, the job logic shouldn't know anything about the job's state data, and likewise the job runner shouldn't care about the job data that the logic is executing on - other than persisting and providing it for the job again later. However there should be some API hooks for a Job to at least highlight to its runner certain things; eg whether it's broken, or if it has further steps to process, logging messages. 

interface ManagedJob {
    public function getMaxRetries();
    public function isComplete();
    public function getCurrentStep();
    public function getTotalSteps();
}

Again, a trait could be used to provide a base level implementation.

This has some possible implications for _how_ a job that has state is created and added onto a queue. Eg

$this->runner->queue(new StatefulJob('MyJobClass', $args));

at which point it is the responsibility of the StatefulJob class to provide much of the state management that QueuedJobService::runJob() currently provides, such as persisting data, pausing jobs going over memory limits, etc, and MyJobClass would implement ManagedJob (possibly via a trait). This ultimately keeps the core JobRunner API as lean as possible.

Lastly, when the job needs to be run in the future; either on a regular schedule, or a once-off point in time (think embargo/expiry). Based on jobs in existing projects, I think almost half the job types I make use of are those that are bits of functionality that are meant to be executed "later", based on either state known "now", or the state of some known objects in the future. Yes, scheduled jobs could be considered something that a separate module provides the implementation thereof, however it's such a common use-case that I don't think a core API can ignore it; counter to that, I'm not sure that it should be a core responsibility to provide an implementation of it.

// a job that should be scheduled; a separate queue runner that handles these would be needed
interface ScheduledJob {
    public function nextRunTime();
}

For a once-off future job, you could have

$this->runner->queue(new FutureJob(['at' => '2017-12-31 23:59:59', 'job' => 'MyJobClass']));

whereby FutureJob would implemented ScheduledJob and figure out when the underlying "normal" job should be run.

Here though, the actual JobRunner would likely need to contain the logic for determining if/when those future jobs should be run; again, so long as it meets the defined interface, I don't see it as being an issue. Interestingly with the way I implemented the queued jobs worker that operates with SQS, the scheduled jobs are now handled by a separate message handler that only looks at "Scheduled" jobs, separately from the "run asap" jobs worker.

(As an aside; for scheduling jobs, I've used https://github.com/silverstripe-australia/silverstripe-schedulizer for defining a job's schedule; the oncomplete of a job simply looks up a defined schedule with the same name as itself, then gets the next run time. This is likely too much for most "simple" things. Theoretically this scheduling mechanism can likely be covered with its own interface, with a simple implementation that looks up information from yml config, and a more complex user-managed implementation like this as an alternative.)

Anyway - from a consumer developer perspective, I think the specifics of which runner is used should be mostly transparent, at least at the API surface level. Could a base job runner implementation refer to a list of job runners that all have the opportunity to accept a job, eg

class MultiRunner implements JobRunner {

    public $runners;

    public function queue(Job $j) {
        foreach ($this->runners as $runner) {
            $status = $runner->queue($j);
            if ($status && $status->getStatus() > 0) {
                continue;
            }
        }

        if (!$status) {
            // just execute immediately
        }

        return $status;
    }
}

Of course, what happens in the case of a ManagedJob and ScheduledJob? Which queue should take it ? Is being "aware" of the other queue type in that scenario worth it?

Tangentially, but perhaps related, since we're talking about executing isolated pieces of logic. What about support for executing logic as a _different_ user to that currently logged in? By which I mean

$runner->runAs($job, $member);

would execute that job in a security context of $member. I know there's modules around for switching users, but there's something about them that scares me...

Along the same lines

$runner->inTransaction($job);

would execute $job atomically; sure, a developer could explicitly write the ->beginTransaction() and ->endTransaction() statements themselves, but there may be other stateful considerations that can be taken into account.

nyeholt picture nyeholt on 25 Jan 2017

Thanks for the brain-dump, @nyeholt! I think the idea is coming together.

A few points:

Callables vs Job objects

I think that accepting both an implementation of Job and a callable to Queue complicates the interface. Anywhere we've allowed a range of different types to be passed to a method for convenience, we've tended to later regret and refactor away.

One way of dealing with this is just to except that developers will wrap a callback in a CallableJob:

$runner->queue(new CallableJob(function() { mail("[email protected]", "What a boring email!"); }));

If that's too hard, we could always have a pair of queue methods on the runner interface, and provide a CallableJob class and trait-based implementation of queueCallback.

$runner->queueJob(new CallableJob(function() { mail("[email protected]", "What a boring email!");
$runner->queueCallback(function() { mail("[email protected]", "What a boring email!"); });

I guess that these are both lean enough to belong to the base implementation.

Serialised objects

I'm opposed to the idea of using classname as the serialisation of a job object, as it limits you writing code that builds out one of a number of different kinds of jobs by populating its parameters.

Yes, I think that serialisation can be confusing, but what's wrong with relying on PHP's underlying serialization implementation, with liberal use of the Serializable interface?

Perhaps part of improving the Serialization experience would be to look at how we serialize things elsewhere in the framework. For example, with DataObjects – should we serialize the whole content, or should we expect to just serialize the class/ID and re-fetch these from the database on unserialization? In any case, giving developers some guidelines about how to construct their jobs (don't store rich objects in your properties; if you have to, implement Serializable), might be less friction than rolling our own serialization scheme.

Unique IDs needn't be hard:

$key = md5(serialize($job));

JobStatus vs JobIdentifier

I like the name JobIdentifier – it's more accurate. But do we need a JobStatus object, or can JobRunner::getStatus() simply return an integer?

JobIdentifier::getIdentifier() presumably returns a serialized format of a JobIdentifier of some kind. Again – wouldn't it make sense to rely on PHP's built-in serialization scheme? This would have flow-on benefits, e.g. if you put a JobIdentifier object into your session, it would be handled automatically.

Scheduled jobs

It's been my general experience that the sysadmin and not the developer is in control of exactly when a regularly occurring job runs. A developer has a level of control more like "these are our daily tasks" and "these are the tasks that need to be run as often as is practical".

In SS3 implementations these often take the form of BuildTasks that are manually transcribed into cronjob sake commands.

There are, of course, applications that do require specific developer-controlled timing, but I don't think these are part of the core API. It's also worth nothing that you'd probably want something more sophisticated than a queue for handling complex scheduling behaviour, for performance reasons.

Managed jobs and job state

Additionally, I think the separation of job state (as far as any external management module cares about it) should be stored separately from job data / parameters. Effectively, the job logic shouldn't know anything about the job's state data, and likewise the job runner shouldn't care about the job data that the logic is executing on - other than persisting and providing it for the job again later.

I agree with everything except the bolded statement: I agree that job state and job parameters should be kept separate. However, if we let the jobs access their own state data, then a lot of higher order features can be implemented as Job Decorators, rather than needing special handling in the runner. In other words, it would make the core API much more lego-like and that's something I'd really like to see.

Exactly how the API is structured would need a bit of work, but I see the core of it as being that jobs can finish in 1 of 3 states:

  • Succcess
  • Failure
  • Requeue, with amended state parameters

This would allow for retries and multi-part jobs. It would also allow for a scheduler, but that would probably involve an inefficient amount of re-queuing (you could keep requeuing a job until the scheduled time had arrived).

Jobs queueing jobs queueing jobs

It's not mentioned above, but I think that passing $runner to Job::run() would be helpful, as it would let jobs queue other jobs without needing to refer back to the global Injector. This would provide a good pattern for doing multi-step jobs: initiate a master job that queues a bunch of other atomic jobs.

Run arguments or properties in the job object?

You've suggested that we could have arguments to a job that are passed in run(). The other way of doing such things is to load the relevant arguments as properties of the job. In other words, the difference between this:

$runner->queue(new MyJob(), [ "some" => "value" ]);

And this:

$runner->queue(new MyJob([ "some" => "value" ]));

Adding arguments support broadens the API, and so my first instinct is to ask "what's the value? is it justified?" If we're looking to avoid complex serialization rules for jobs, then this might make sense as you could simply persist the classname of the job. However, I feel that this will end up awkwardly limited (and I've suggested above that we promote the use of Serializable)

Transactional and Authenticated jobs

Could we do these as job decorators?

$runner->queue(new JobAsMember($member->ID, new MyJob()));
$runner->queue(new TransactionJob(new MyJob()));

// Por que no los dos?
$runner->queue(new TransactionJob(new JobAsMember($member->ID, new MyJob())));
sminnee picture sminnee on 31 Jan 2017

Much more succinct this time...

accepting both an implementation of Job and a callable to Queue complicates the interface.

The java dev in me disregards this notion, but the sane part of me agrees :). So agreed, expecting someone to wrap their closure before method call is reasonable, it forces someone to think about what they're doing.

serialisation can be confusing, but what's wrong with relying on PHP's underlying serialization implementation

Totally agree with you on that; it's more from the perspective of the feedback from people using queuedjobs, where the concept of persistent state seems to cause issues for people. The more I've thought through things though, the more likely it'll be that the majority of users will be a case of not caring about that re-entrant state management side of things, and that more complex job-runner-jobs can be created to actually handle that if desired, outside the core API.

JobIdentifier::getIdentifier() presumably returns a serialized format of a JobIdentifier

Thinking from a persisted job perspective, my thinking was more that the stored JobStatus object would have a column which was related to the identifier in some way; whether that's an integer, hash, uniqueid of some sort. It may even reference an external system in some way (no concrete thought there, just speculation :). So the identifier string may be 0122-3bx4, or it could be sqs://queuename, or 1; the concrete JobIdentifier class could be responsible for interpreting this.

I'd see JobRunner::getStatus() returning a JobStatus object, which would then return an int from getStatus() (would probably be clearer calling that getState() though...)

that the sysadmin and not the developer is in control of exactly when a regularly occurring job runs

But that's because it has needed a sysadmin to create a cronjob for running the job/task, and the answer to the question "when does this need to run" is "all the time!" :). My point being that as a developer, I don't want to have to deal with sysdmins (either directly by asking them to do something, or by thinking outside the code space) - I want to be able to say "run this bit of code at 10:30 tomorrow morning because the user has requested a page to be published then".

sophisticated than a queue for handling complex scheduling behaviour

Agreed - but the outcome of that scheduling logic will be a job on a queue for a certain time in the future.

if we let the jobs access their own state data, then a lot of higher order features

I see what you mean - my main concern being that the job logic (ie publish a tree of pages) shouldn't necessarily be able to freely change the state data of how it exists on a queue somewhere.

I think that passing $runner to Job::run()

The more I think about it, the more I agree and prefer run() has a fixed interface (ie accepting a Logger and Queue), particularly for anonymous jobs which I feel would become much more useful.

You've suggested that we could have arguments to a job that are passed in run().

See above :). Agreed, a fixed interface to implement for run() is likely to provide a less confusing approach.

Transactional and Authenticated jobs

Yep!

nyeholt picture nyeholt on 17 Feb 2017

OK, thanks Marcus! It looks like we're in broad agreement and I'll go re-write the original post.

sminnee picture sminnee on 19 Feb 2017

So this, isn't going to make the beta1 cut, but in principle it should be possible to do this as a minor release change, so long as we provide an adapter from BuildTasks to the new Job interface.

sminnee picture sminnee on 29 Jun 2017

I'm adding this to the 5.0 milestone to put it back on the radar, our window of opportunity for agreeing on an API is about three months - then we'll need to build the core plus rewrite anything that should rely on it in time for 5.0 in Q4/2018

chillu picture chillu on 18 Feb 2018
👍1

Can we move this to rfc/accepted?

robbieaverill picture robbieaverill on 15 Feb 2019

Moved to rfc/accepted. Suggested to @Cheddam that he uses this as inspiration during hack-week :-)

sminnee picture sminnee on 11 Mar 2019
Was this page helpful?
0 / 5 - 0 ratings

Related issues

zanderwar picture zanderwar  Âˇ  6Comments
chillu picture chillu  Âˇ  5Comments
dnsl48 picture dnsl48  Âˇ  6Comments
maxime-rainville picture maxime-rainville  Âˇ  3Comments
Martimiz picture Martimiz  Âˇ  5Comments