Skip to content

[Messenger] Add SKIP LOCKED to the query that retrieves messages #52639

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

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

hgraca
Copy link

@hgraca hgraca commented Nov 17, 2023

Q A
Branch? 7.1
Bug fix? no
New feature? yes
Deprecations? no
Issues -
License MIT

The current SELECT ... FOR UPDATE retrieves rows, using the index to find and lock the retrieved rows.
This means that when we have more than one consumer, while one consumer is locking those rows, the whole index is locked thus other queries (consumers) will be put on hold.
While with a small table this might not be noticeable, as the table grows the meddling with the index becomes slower and the other consumers have to wait more time, eventually making the MQ inoperable.

The SKIP LOCKED addition will allow other consumers to query the table and get messages immediately, ignoring the rows that other consumers are locking.

@carsonbot carsonbot added this to the 7.1 milestone Nov 17, 2023
@hgraca hgraca force-pushed the messenger/add_skip_locked branch 7 times, most recently from 88fce4e to 1314061 Compare November 18, 2023 11:08
@hgraca hgraca marked this pull request as draft November 18, 2023 11:10
@hgraca hgraca force-pushed the messenger/add_skip_locked branch from 1314061 to a36c205 Compare November 18, 2023 11:40
@hgraca hgraca marked this pull request as ready for review November 18, 2023 12:24
new OraclePlatform(),
'SELECT w.id AS "id", w.body AS "body", w.headers AS "headers", w.queue_name AS "queue_name", w.created_at AS "created_at", w.available_at AS "available_at", w.delivered_at AS "delivered_at" FROM messenger_messages w WHERE w.id IN (SELECT a.id FROM (SELECT m.id FROM messenger_messages m WHERE (m.queue_name = ?) AND (m.delivered_at is null OR m.delivered_at < ?) AND (m.available_at <= ?) ORDER BY available_at ASC) a WHERE ROWNUM <= 1) FOR UPDATE',
];
} elseif (class_exists(MySQL57Platform::class)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Oracle?

Copy link
Author

Choose a reason for hiding this comment

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

No, this was already there, it's a way to know if we are using DBAL 4 or not.
Since MySQL57Platform is deprecated, it will not be included in DBAL4 anymore.
I guess we could/should put this in a method with a meaningful name...
I can do that if/when I have time, otherwise, I think it's fine to leave it as is since it was already like this.

@carsonbot carsonbot changed the title Add "SKIP LOCKED" to the query that retrieves messages [Messenger] Add "SKIP LOCKED" to the query that retrieves messages Nov 20, 2023
@OskarStark OskarStark changed the title [Messenger] Add "SKIP LOCKED" to the query that retrieves messages [Messenger] Add SKIP LOCKED to the query that retrieves messages Nov 20, 2023
@nicolas-grekas nicolas-grekas force-pushed the messenger/add_skip_locked branch from 8d89fce to b4fe683 Compare November 21, 2023 13:12
@nicolas-grekas
Copy link
Member

Thank you @hgraca.

@nicolas-grekas nicolas-grekas merged commit 06a85e0 into symfony:7.1 Nov 21, 2023
@hgraca
Copy link
Author

hgraca commented Nov 21, 2023

You are most welcome @nicolas-grekas! Thank you for Symfony! :)

Btw, do you think you can help me get this other PR merged as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants