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

Migrate OnlineDDL commands to vtctldclient #13712

Closed
9 tasks done
ajm188 opened this issue Aug 3, 2023 · 6 comments
Closed
9 tasks done

Migrate OnlineDDL commands to vtctldclient #13712

ajm188 opened this issue Aug 3, 2023 · 6 comments
Assignees
@shlomi-noach @ajm188
Labels
Component: Online DDL Online DDL (vitess/native/gh-ost/pt-osc) Type: Feature Request
Projects
Vtctld Service

Comments

@ajm188
Copy link
Contributor

ajm188 commented Aug 3, 2023 •
edited

@ajm188 ajm188 created this issue from a note in Vtctld Service (In progress) Aug 3, 2023
@ajm188 ajm188 added Component: Online DDL Online DDL (vitess/native/gh-ost/pt-osc) Type: Feature Request labels Aug 3, 2023
This was referenced Aug 27, 2023
Merged
Merged
@shlomi-noach
Copy link
Contributor

I'm having second thoughts about throttle and unthrottler. Following v17 changes, these commands should not make it to the tabletmanager at all. In fact, if you issue alter vitess_migration throttle all command in vtgate, your query gets handled differently than an alter vitess_migration complete all: vtgate will run specialized code to UpdateSrvKeyspace in the throttle case, where for complete it will just send the query to the tablet.

I'd rather cut hard at this time and simply not implement throttle and unthrottle. What was originally conceived as consistent queries/behavior is now very different, and today I would now add these commands in vtctl nor in vtgate.

This was referenced Aug 30, 2023
Closed
Closed
Closed
Closed
Closed
Closed
@shlomi-noach
Copy link
Contributor

@ajm188 what are your thoughts?

@ajm188
Copy link
Contributor Author

ajm188 commented Sep 4, 2023

vtgate will run specialized code to UpdateSrvKeyspace in the throttle case, where for complete it will just send the query to the tablet.

I'd rather cut hard at this time and simply not implement throttle and unthrottle. What was originally conceived as consistent queries/behavior is now very different, and today I would now add these commands in vtctl nor in vtgate.

So, is the "right" way now to call UpdateSrvKeyspace with vtctdlclient? Or to call UpdateThrottlerConfig? I don't see why the vtctld rpc can't be a wrapper to pass specific values to those same functions (i.e. vtctldclient Throttle <uuid> is equivalent to vtctldclient UpdateThrottlerConfig <payload relating to the specific schema migration with "uuid">)

@shlomi-noach
Copy link
Contributor

The "right" was is to call UpdateThrottlerConfig. Yes, I can do that redirection.

@ajm188
Copy link
Contributor Author

ajm188 commented Sep 5, 2023

Sounds good!

@shlomi-noach shlomi-noach mentioned this issue Sep 5, 2023
Merged
4 tasks
@shlomi-noach
Copy link
Contributor

All commands have been migrated and this issue is complete.

Vtctld Service automation moved this from In progress to Done Sep 12, 2023
@mattlord mattlord mentioned this issue Oct 11, 2023
Merged
4 tasks
@BrewTestBot BrewTestBot mentioned this issue Nov 6, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@shlomi-noach shlomi-noach

@ajm188 ajm188

Labels
Component: Online DDL Online DDL (vitess/native/gh-ost/pt-osc) Type: Feature Request
Projects
Vtctld Service
  
Done
Milestone
No milestone
Development

No branches or pull requests
2 participants
@shlomi-noach @ajm188