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.

Sign up for GitHub

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

Jump to bottom

non-unique vindex routing in update and delete query #9554

Merged
merged 6 commits into from
Jan 28, 2022
Merged

non-unique vindex routing in update and delete query #9554

merged 6 commits into from
Jan 28, 2022

Conversation

harshit-gangal
Copy link
Member

@harshit-gangal harshit-gangal commented Jan 24, 2022 •
edited
Loading

Description

This PR adds supported for extracting routing information from update/delete query and using non-unique lookup vindex if available and having lowest cost among other options.

Related Issue(s)

Checklist

  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

@harshit-gangal
test: added non-unique vindex column to multicol table
fc9343a 
Signed-off-by: Harshit Gangal <harshit@planetscale.com>
@harshit-gangal
feat: allow non-unique vindex for routing selection in dml queries
c4d6d1b 
Signed-off-by: Harshit Gangal <harshit@planetscale.com>
@harshit-gangal
test: delete cases added for non-unique lookup vindex selection
b526973 
Signed-off-by: Harshit Gangal <harshit@planetscale.com>
@harshit-gangal
feat: dml engine to allow multi destination query for equal plan to a…
516c73f 
…ccomodate non-unique lookup

Signed-off-by: Harshit Gangal <harshit@planetscale.com>
@harshit-gangal
feat: consistent lookup to use commit order based on the input query …
dc63586 
…to vtgate instead of fixed pre commit order

Signed-off-by: Harshit Gangal <harshit@planetscale.com>
@harshit-gangal harshit-gangal marked this pull request as ready for review January 28, 2022 09:36
GuptaManan100
GuptaManan100 approved these changes Jan 28, 2022
View reviewed changes
Copy link
Member

@GuptaManan100 GuptaManan100 left a comment

Choose a reason for hiding this comment

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

Everything looks good to me! Is there a way to add an end to end test for this? I know it is hard since we won't know which vindex was used for routing, but we could do something like try deleting and check which all shards received that query

frouioui
frouioui approved these changes Jan 28, 2022
View reviewed changes
Copy link
Member

@frouioui frouioui left a comment

Choose a reason for hiding this comment

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

Looks good to me

@harshit-gangal
Copy link
Member Author

Everything looks good to me! Is there a way to add an end to end test for this? I know it is hard since we won't know which vindex was used for routing, but we could do something like try deleting and check which all shards received that query

already on it.

@harshit-gangal
test: added e2e test for non-unique dml routing
a6fef2a 
Signed-off-by: Harshit Gangal <harshit@planetscale.com>
@harshit-gangal harshit-gangal merged commit 7d3a49f into vitessio:main Jan 28, 2022
@harshit-gangal harshit-gangal deleted the dml-nonunique branch January 28, 2022 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GuptaManan100 GuptaManan100 GuptaManan100 approved these changes

@frouioui frouioui frouioui approved these changes

@deepthi deepthi Awaiting requested review from deepthi

@systay systay Awaiting requested review from systay systay is a code owner

@aquarapid aquarapid Awaiting requested review from aquarapid

Assignees
No one assigned
Labels
Component: Query Serving Type: Feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@harshit-gangal @frouioui @GuptaManan100