Update MakeQueueTenantAwareAction.php

[masterix21]

The PR is a follow-up of issue #422

[freekmurze]

Ok, I think this would solve the problem that a stale version of the tenant is used in some jobs.

Is there any way to add a test for this that ensure that the tenant model is in fact reloaded between jobs?

[masterix21]

@freekmurze, if your check is ok, I will merge the PR.

[freekmurze]

Looks good to me 👍

[masterix21]

Well, merged. Thanks.

[moisish]

This PR has introduce a breaking change as per my bug here #446
Following the initial conversation #422 I believe changes on the Tenant model should be handle differently and not with the solution introduced in 3.0.2

public static function boot()
{
    parent::boot();

    static::updated(function ($item) {
        $containerKey = config('multitenancy.current_tenant_container_key');

        app()->forgetInstance($containerKey);

        app()->instance($containerKey, $this);
    });
}

[masterix21]

@moisish, sorry, but using your logic, you will overwrite the current tenant if updates another one.

[moisish]

@masterix21 You are right. We are still facing the issue tho so we need to find a solution

[danijelk]

Adding our issue to the list as well.

We use 'queues_are_tenant_aware_by_default' => false, and everything was fine <3.0.0. In 3.0.2 all jobs forget current tenant and the rest of the assertions fail as it forgets tenant for the rest of the code running after a dispatch.

Setting 'queues_are_tenant_aware_by_default' to true - works fine in 3.0.1.

In 3.0.2 once any dispatch_now is done the makeCurrent doesn't seem to reconnect (caused by the forget and re-set) - tenant database is now empty, I am yet to find the root cause.

Looking at the original problem I see why this was changed, I would have preferred an interface JobShouldRefreshTenant which would trigger a get/forget when checking with implementsInterface. This is cause we never change any tenant data and would like to avoid a forget/reset overhead if there is no reason to do it.

--- UPDATE

issue for test db being empty - it's transactions, as per documentation

Event::listen(MadeTenantCurrentEvent::class, function () {
            $this->beginDatabaseTransaction();
});

This won't work with 3.0.2 cause of the new forget & makecurrent flow.

[benddailey]

I believe #438 should be reverted based on the reports of many. I found this thread while dealing with failed Broadcast and failed laravel-medialibrary conversion jobs. I realized the issue is caused because of the long running worker queue. I ended up on rather simple fix for this issue. Switch production to run worker:listen instead of worker:queue. There is a very small performance penalty for booting the framework for each job but it is better to get the right and proper result than having failed jobs or broadcast messages that don't go anywhere because the have the wrong config parameters.

I switched my supervisor configuration for my worker /etc/supervisor/conf.d/wildcard_auctionit_io-worker.conf as seen below:

[program:wildcard_auctionit_io-worker]
process_name=%(program_name)s_%(process_num)02d
#command=/usr/bin/php /var/www/wildcard_auctionit_io/artisan queue:work --sleep 1 --timeout 120
command=/usr/bin/php /var/www/wildcard_auctionit_io/artisan queue:listen --sleep 1 --timeout 120
numprocs=4
autostart=true
autorestart=true
user=www-data
redirect_stderr=true
stdout_logfile=/var/www/wildcard_auctionit_io/storage/logs/worker.log

Maybe the documentation should be updated to mention running queue:listen instead of queue:work?

[mbardelmeijer]

I agree that #438 should be reverted and moved to a major release (with additional fixes). We just spent a few hours debugging an edge case that also fell back to this.

I agree that the changes made should still be relevant in an upcoming major release but seem to have breaking changes on a minor release.