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

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

Merged
merged 1 commit into from
Jan 29, 2020

Conversation

themsaid
Copy link
Member

@themsaid themsaid commented Jan 29, 2020

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
Member

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
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
Copy link

Krunch commented Feb 4, 2020

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
Copy link
Member Author

themsaid commented Feb 4, 2020

@Krunch locks are skipped for SqlServer, same as Sqlite

@Krunch
Copy link

Krunch commented Feb 4, 2020

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

@Krunch
Copy link

Krunch commented Feb 4, 2020

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
Copy link
Contributor

Krisell commented Feb 5, 2020

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';

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
Copy link

Krunch commented Feb 5, 2020

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