Skip to content

[6.x] Use SKIP LOCKED for mysql 8.1 and pgsql 9.5 queue workers - #31287

Merged
taylorotwell merged 1 commit into
laravel:6.xfrom
themsaid:pr3482
Jan 29, 2020
Merged

[6.x] Use SKIP LOCKED for mysql 8.1 and pgsql 9.5 queue workers#31287
taylorotwell merged 1 commit into
laravel:6.xfrom
themsaid:pr3482

Conversation

@themsaid

@themsaid themsaid commented Jan 29, 2020

Copy link
Copy Markdown
Member

Using SKIP LOCKED on MySQL 8.0.19 I was able to get 516,783 jobs processed by 50 workers in 25 minutes on a 3GB RAM - 1 CPU core DigitalOcean server. No deadlocks.

Running the same test without SKIP LOCKED I started getting deadlocks after a few minutes.

$databaseEngine = $this->database->getPdo()->getAttribute(PDO::ATTR_DRIVER_NAME);
$databaseVersion = $this->database->getPdo()->getAttribute(PDO::ATTR_SERVER_VERSION);

if ($databaseEngine == 'mysql' && version_compare($databaseVersion, '8.0.1', '>=') ||

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kinda feels like a hack we need to look at the engine here. Feels like this should be the responsibility of the database component.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is a hack :)

@Krunch

Krunch commented Feb 4, 2020

Copy link
Copy Markdown

This breaks compatibility with MSSQL server @taylorotwell. It does not support the "SELECT FOR UPDATE" syntax. Whenever I now try to run the queue with queue:work I am getting this error:

Illuminate\Database\QueryException SQLSTATE[42000]: [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Line 1: FOR UPDATE clause allowed only for DECLARE CURSOR. (SQL: select top 1 * from [jobs] FOR UPDATE where [queue] = default and (([reserved_at] is null and [available_at] <= 1580841485) or ([reserved_at] <= 1580841395)) order by [id] asc)

Queues are unusable in MSSQL now because of this commit. It should be reverted until a proper solution that covers all currently supported databases is implemented.

@themsaid

themsaid commented Feb 4, 2020

Copy link
Copy Markdown
Member Author

@Krunch locks are skipped for SqlServer, same as Sqlite

@Krunch

Krunch commented Feb 4, 2020

Copy link
Copy Markdown

Not with this commit @themsaid. If I put ->lockForUpdate() back instead of this commit ->lock($this->getLockForPopping()) the MSSQL Queues work again.

@Krunch

Krunch commented Feb 4, 2020

Copy link
Copy Markdown

With this commit the SQL query generated on MSSQL is this: select * from [jobs] FOR UPDATE where [queue] = ? and (([reserved_at] is null and [available_at] <= ?) or ([reserved_at] <= ?)) order by [id] asc

Before this commit: select * from [jobs] with(rowlock,updlock,holdlock) where [queue] = ? and (([reserved_at] is null and [available_at] <= ?) or ([reserved_at] <= ?)) order by [id] asc

As you can see the locking part is drastically different. @themsaid

@Krisell

Krisell commented Feb 5, 2020

Copy link
Copy Markdown
Contributor

We're experiencing the same problem with SQLServer after updating to 6.14, the queue-workers crash and the queue is left unprocessed. We reverted back to 6.13.1 which works.

return 'FOR UPDATE SKIP LOCKED';
}

return 'FOR UPDATE';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'FOR UPDATE' does not seem to work in MSSQL while not in a cursor.

@Krunch

Krunch commented Feb 5, 2020

Copy link
Copy Markdown

I opened Issue #31368 for this specific bug.

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.

7 participants