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

Do not run expensive query in save_task if SAVE_LIMIT is 0 #639

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rubendv
Copy link

@rubendv rubendv commented Dec 16, 2021

Check if SAVE_LIMIT is bigger than 0 before running any queries.

On MS SQL Server, the select_for_update() query below will take a ROWLOCK,UPDLOCK on every row in the Task table. This happens because it locks every row it needs to look at to return the result of the query. Since there is no index on the default sorting column "stopped", it has to do a full table scan, and hence it will take rowlocks on each row in the table.
On other databases, it will still do a full table scan, but without the excessive locking.
If SAVE_LIMIT is 0 the result of this query is unused, so by moving the check a level higher we avoid running this expensive query.

It might also be a good idea to optimize the query if SAVE_LIMIT is not 0, for example by putting an index on the stopped column, or by ordering this query by pk instead of the stopped column.

On MS SQL Server, the select_for_update() query below will take a
ROWLOCK,UPDLOCK on every row in the Task table. This happens because it locks
every row it needs to look at to return the result of the query.
Since there is no index on the default sorting column "stopped", it has
to do a full table scan, and hence it will take rowlocks on each row
in the table.
On other databases, it will still do a full table scan, but without the
excessive locking.
If SAVE_LIMIT is 0 the result of this query is unused, so by moving the
check a level higher we avoid running this expensive query.
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.

None yet

1 participant