laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.54k stars 11.02k forks source link

When executing a job within a batch, occasionally we run into an error "There is no active transaction" #44152

Closed kmizzi closed 2 years ago

kmizzi commented 2 years ago

Description:

When executing a job within a batch, we run into an active transaction error triggered on /vendor/laravel/framework/src/Illuminate/Database/Concerns/ManagesTransactions.php:46

The specific line reads: $this->getPdo()->commit();

It is trying to commit the transaction, but based on what we are doing in the job (performing inserts, updates, creates), this conflicts. We did have one transaction in the job but even after commenting that out, the error persisted.

Steps To Reproduce:

Specifically we created a chained batch job, there are certain ALTER, CREATE, and DROP SQL statements we are running which can't be in a transaction (but being within a batch forces it to be wrapped in a transaction). We know there are at least 2 failure points that are causing this but were only able to find one (provided in code sample)

Code sample:

public function handle()
{
    $chained_jobs = array_map(function ($chunk) {
        static $delay = 0;

        $job = (new GetOrderItemsJob($chunk))->delay($delay);
        $delay = 20;

        return $job;
    }, array_chunk($this->orderRepository->getActiveItemlessOrderIds()->toArray(), 10));

    Bus::batch([
        $chained_jobs
    ])->name('GetAmazonOrderItems')->dispatch();
}

Problematic code within the job:

DB::connection($this->databaseConnection)->statement("ALTER TABLE $this->temporaryTable DROP $column");

driesvints commented 2 years ago

Please read the docs here to see if they help: https://laravel.com/docs/9.x/queues#jobs-and-database-transactions

kmizzi commented 2 years ago

Thanks @driesvints

I wish this would have worked, but no luck. After changing the queue config to flag after_commit = true for redis, still the same error

I tried queue:restart, cache:clear, config:clear, and clearing redis (flushall) just to be sure

driesvints commented 2 years ago

Heya, thanks for reporting.

We'll need more info and/or code to debug this further. Can you please create a repository with the command below, commit the code that reproduces the issue as separate commits on the main/master branch and share the repository here? Please make sure that you have the latest version of the Laravel installer in order to run this command. Please also make sure you have both Git & the GitHub CLI tool properly set up.

laravel new bug-report --github="--public"

Please do not amend and create a separate commit with your custom changes. After you've posted the repository, we'll try to reproduce the issue.

Thanks!

kmizzi commented 2 years ago

Done: https://github.com/kmizzi/laravel-bug-report

driesvints commented 2 years ago

I managed to boil it down to the following reproducible example:

Bus::batch([function () {
    DB::statement('CREATE TEMPORARY TABLE IF NOT EXISTS temporary_table (
        `id` int(11),
        `ColumnToDrop` varchar(200)
    ) ENGINE=INNODB');

    DB::statement("ALTER TABLE temporary_table DROP `ColumnToDrop`");
}])->dispatch();

What happens is actually that jobs within batches are wrapped in a transaction so they're easy to rollback when something goes amis. The reason this fails is because you're executing implicit commits within the job. See the docs on that here: https://laravel.com/docs/9.x/database#implicit-commits-in-transactions

By issuing the temporary table execution, you're messing with the transaction level and Laravel remains unaware of this. Therefor it's not possible to execute such queries within batches.

I've added a note to the docs about this: https://github.com/laravel/docs/pull/8234