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

[8.x] New JobRetrying event dispatched #39097

Merged
merged 3 commits into from
Oct 7, 2021
Merged

[8.x] New JobRetrying event dispatched #39097

merged 3 commits into from
Oct 7, 2021

Conversation

masterix21
Copy link
Contributor

The PR introduces the new event JobRetrying, dispatched calling the queue:retry command.

For example, it's a magic bullet for the models strictly related to a dynamic tenant connection. To learn more about it, see here: spatie/laravel-multitenancy#259 (comment)

@taylorotwell
Copy link
Member

Can you show me what you plan to do in the listener for this event?

@GrahamCampbell GrahamCampbell changed the title New JobRetrying event dispatched [8.x] New JobRetrying event dispatched Oct 5, 2021
@masterix21
Copy link
Contributor Author

Sure.

The necessity of an event JobRetrying started using the package spatie/laravel-multitenancy. In the multitenancy context, many applications use models and database connections with dynamic parameters.

For example, imagine a situation like so:

class User extends Model {
    use UsesTenantConnection; // It configures the tenant connection dynamically
}

class TenantUserJob {
    public function __construct(TenantUser $user) {
      // ...
    }
}

When you dispatch the job for the first time, the job has the connection configured and works well. But, what happens when the job is unserialized from the retry command? you won't have a connection configured, and an event like JobRetrying could help the developers with a code like so:

app('events')->listen(JobRetrying::class, function (JobRetrying $event) {
    if (! array_key_exists('tenantId', $event->payload())) {
        return;
    }

    $this->findTenant($event)->makeCurrent();
});

@taylorotwell
Copy link
Member

I don't understand how that solves your problem. What does makeCurrent do?

@masterix21
Copy link
Contributor Author

masterix21 commented Oct 6, 2021

makeCurrent executes many tasks, and one of these - the most important - is:

// `$tenantConnectionName` contains the tenant connection name configured by the developers

// `$databaseName` is the database name that `makeCurrent` takes from the landlord database

config([
    "database.connections.{$tenantConnectionName}.database" => $databaseName,
]);

app('db')->extend($tenantConnectionName, function ($config, $name) use ($databaseName) {
    $config['database'] = $databaseName;

    return app('db.factory')->make($config, $name);
});

DB::purge($tenantConnectionName);

It reconfigures the tenant connection using a multi-database approach.

Another important task executes by makeCurrent is MakeQueueTenantAwareAction, task that inject in the dispatched job the tenant-id reference like so:

app('queue')->createPayloadUsing(function ($connectionName, $queue, $payload) {
    return ['tenantId' => Tenant::current()?->id];
});

Finally the task listen for the Laravel event JobProcessing like so:

app('events')->listen(JobProcessing::class, function (JobProcessing $event) {
    Tenant::find($event->job->payload()['tenantId'])?->makeCurrent();
});

Now, take a look here:

class User extends Model {
    use UsesTenantConnection; // It configures the tenant connection dynamically, with the connection name previously seen
}

class TenantUserJob {
    public function __construct(TenantUser $user) {
      // ...
    }
}

When a job fails, the job is serialized. Using the command queue:retry, you can't unserialize the job TenantUserJob, because the model TenantUser uses a connection that won't exist (because isn't configured using the makeCurrent), but it could be solved by listening to a JobRetrying event in the same way of JobProcessing.

@taylorotwell
Copy link
Member

taylorotwell commented Oct 6, 2021

But, how does this help you if that stuff happens in the JobRetry command - the job is then pushed out onto the queue and executed by an entirely different process?

I guess the createPayloadUsing stuff? That affects pushRaw?

@laravel laravel deleted a comment from bonroyage Oct 6, 2021
@masterix21
Copy link
Contributor Author

Looking to the Laravel RetryCommand, the refreshRetryUntil unserialize the instance from a payload.

image

What happens if you try to unserialize a model that has a reference to a connection that doesn't exist? It fails, and you can't retry the job: SQLSTATE[3D000]: Invalid catalog name: 1046 No database selected

Using JobRetry, we can do the same of JobProcessing, associating the right database connection before the unserialization, like so:

app('events')->listen(JobRetry::class, function (JobRetry $event) {
    Tenant::find($event->job->payload()['tenantId'])?->makeCurrent();
});

@taylorotwell
Copy link
Member

I renamed the event to JobRetryRequested since it is fired from the CLI when the job is initially requested to be retried and the job is not actually on the queue yet.

@taylorotwell taylorotwell merged commit 65702d2 into laravel:8.x Oct 7, 2021
@masterix21
Copy link
Contributor Author

Thanks

chu121su12 pushed a commit to chu121su12/framework that referenced this pull request Oct 9, 2021
* New JobRetrying event dispatched

* Style fixes

* formatitng

Co-authored-by: Taylor Otwell <taylorotwell@gmail.com>
@masterix21 masterix21 deleted the feature-hydrate-failed-jobs branch October 11, 2021 08:30
victorvilella pushed a commit to cdsistemas/framework that referenced this pull request Oct 12, 2021
* New JobRetrying event dispatched

* Style fixes

* formatitng

Co-authored-by: Taylor Otwell <taylorotwell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants