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

Suggestions for online DDL query syntax #7604

Closed
shlomi-noach opened this issue Mar 4, 2021 · 11 comments
Closed

Suggestions for online DDL query syntax #7604

shlomi-noach opened this issue Mar 4, 2021 · 11 comments
Assignees
Labels
Component: Query Serving Type: Enhancement Logical improvement (somewhere between a bug and feature)

Comments

@shlomi-noach
Copy link
Contributor

shlomi-noach commented Mar 4, 2021

We wish to supply SQL syntax for Online DDL (and possibly align that with VSCHEMA/VINDEX syntax). Right now it is only possible to submit a migration via SQL, but not to track/control it. Tracking/control are possible via vtctl.

In this issue we propose syntax that is SQL/MySQL idiomatic. We need syntax for:

Suggestion 1

SHOW vitess_migrations; -- shows everything
SHOW vitess_migrations LIKE '24213daa_7b26_11eb_b423_f875a4d24e90';
SHOW vitess_migrations WHERE migration_status='failed'; -- single KEY='VALUE'
SHOW vitess_migrations WHERE mysql_table='my_table'; -- single KEY='VALUE'
SHOW vitess_migrations WHERE started_timestamp > NOW() - INTERVAL 7 DAY; -- Too complex?


ALTER vitess_migration '24213daa_7b26_11eb_b423_f875a4d24e90' RETRY;
ALTER vitess_migration '24213daa_7b26_11eb_b423_f875a4d24e90' CANCEL;
ALTER vitess_migration CANCEL ALL;

REVERT vitess_migration '24213daa_7b26_11eb_b423_f875a4d24e90';

Discussion:

  • We use similar notations to SHOW TABLES... and ALTER TABLE.
  • The SHOW commands are mostly generic. Showing the status of a specific migration is more useful than others, hence specialized LIKE syntax.
  • For RETRY/CANCEL we use ALTER because we are, in fact, changing the migration state, or rather controlling it (anecdotally, behind the scenes this is implemented with UPDATE _vt.schema_migrations)
  • REVERT has its own syntax because:
    • It's not an ALTER of an existing migration: it's the creation of a new migration, with new Job ID, that reverts a previous migration.
    • It's such a special and empowering operation that I feel it deserves its own syntax. I'm a salesperson in disguise.
  • I think a common query is to show "recent" migrations (vtctl OnlineDDL commerce show recent). But in the above, implementation for RECENT is cumbersome (WHERE started_timestamp > NOW() - INTERVAL 7 DAY).

Suggestion 2

Use more verbose syntax:

SHOW vitess_migrations WITH STATUS 'running';
SHOW vitess_migrations WITH STATUS 'complete';
SHOW vitess_migrations WITH STATUS 'failed';
SHOW vitess_migrations FOR TABLE 'my_table';

SHOW FAILED vitess_migrations;
SHOW COMPLETE vitess_migrations;
SHOW RECENT vitess_migrations;

In the above we have specialized syntax for showing migrations per filter.

  • Each reads better
  • We have fine grained control over what can be queried
  • Simplified syntax for RECENT
  • But together there's multiple syntax options to remember

Suggestion 3

Less English, more SQL-ish?

SHOW vitess_migrations FAILED;
SHOW vitess_migrations COMPLETE;
SHOW vitess_migrations RECENT;

Like (2), but putting the commands last.

Soliciting opinions

Tracking issue: #6926

cc @vitessio/ps-vitess

@derekperkins
Copy link
Member

I think I like option 1 the best. If SHOW vitess_migrations sorts by latest timestamp, I think that solves the recent problem.

@shlomi-noach
Copy link
Contributor Author

shlomi-noach commented Mar 4, 2021

Thanks -- yeah, I was wondering whether it should always sort on ID DESC, or started_timestamp DESC, or completed_timestamp DESC, or... should it e ASC?

@shlomi-noach
Copy link
Contributor Author

(normally migrations are executed first-come-first-served, so all three sort methods should return same list. ut if one migration fails, or cancelled, it's possible to pursue the next one, and that's how things get out of order)

@derekperkins
Copy link
Member

I think the most common case would be ORDER BY started_timestamp DESC. completed_timestamp is a little trickier since it'll be NULL while running, and I don't think is compelling enough to choose it.

Would the SHOW operations also support custom ORDER BY and/or LIMIT?

@askdba askdba added Component: Query Serving Severity 4 Type: Enhancement Logical improvement (somewhere between a bug and feature) labels Mar 5, 2021
@shlomi-noach
Copy link
Contributor Author

I think the most common case would be ORDER BY started_timestamp DESC. completed_timestamp is a little trickier since it'll be NULL while running, and I don't think is compelling enough to choose it.

On that note, started_timestamp can also be NULL if the migration hasn't started. I'm leaning towards sorting by id, which is the chronological migration submission order.

Would the SHOW operations also support custom ORDER BY and/or LIMIT?

I'll look into it. Maybe not at first.

@shlomi-noach
Copy link
Contributor Author

#7638 introduces SHOW VITESS_MIGRATIONS query

@shlomi-noach
Copy link
Contributor Author

Would the SHOW operations also support custom ORDER BY and/or LIMIT?

It's going to be complicated, because on a multi sharded keyspace that means sorting/limit on the vtgate side. For now I'm not going to try to address this.

@shlomi-noach
Copy link
Contributor Author

#7656 introduces REVERT VITESS_MIGRATION

@shlomi-noach
Copy link
Contributor Author

#7663 introduces ALTER VITESS_MIGRATION ... statements

@shlomi-noach
Copy link
Contributor Author

We have used suggestion (1) for all syntax.

@shlomi-noach
Copy link
Contributor Author

Solved via #7638, #7656, #7663

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Query Serving Type: Enhancement Logical improvement (somewhere between a bug and feature)
Projects
None yet
Development

No branches or pull requests

3 participants