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

select query on lookup table to lock the row #6620

Merged
merged 12 commits into from
Aug 28, 2020

Conversation

harshit-gangal
Copy link
Member

@harshit-gangal harshit-gangal commented Aug 25, 2020

Fixes: #6604

The select query on a lookup vindex will take exclusive lock if it is a dml query and is executed in a transaction.

Signed-off-by: Harshit Gangal <harshit@planetscale.com>
Signed-off-by: Harshit Gangal <harshit@planetscale.com>
systay and others added 7 commits August 25, 2020 10:23
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Harshit Gangal <harshit@planetscale.com>
…consistenlookup

Signed-off-by: Harshit Gangal <harshit@planetscale.com>
Signed-off-by: Harshit Gangal <harshit@planetscale.com>
Signed-off-by: Harshit Gangal <harshit@planetscale.com>
…PDATE in select query

Signed-off-by: Harshit Gangal <harshit@planetscale.com>
…cursor interface to fake vcursors

Signed-off-by: Harshit Gangal <harshit@planetscale.com>
Copy link
Contributor

@sougou sougou left a comment

Choose a reason for hiding this comment

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

Looks like you're doing an unconditional for update in the select. That may be aggressive and could fail also in read-only cases.

I think you should do it only for DMLs.

Also, I'm wondering if you can leverage an existing e2e test for this. If not, then this is fine.

Signed-off-by: Harshit Gangal <harshit@planetscale.com>
@systay
Copy link
Collaborator

systay commented Aug 26, 2020

@sougou I think we are only adding the for update to DML queries

Signed-off-by: Harshit Gangal <harshit@planetscale.com>
Signed-off-by: Harshit Gangal <harshit@planetscale.com>
@sougou
Copy link
Contributor

sougou commented Aug 26, 2020

@sougou I think we are only adding the for update to DML queries

Oh nvm. I see now :)

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.

Lock wait timeouts/delays for some concurrent update/insert operations involving lookup vindexes
4 participants