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

Support ALTER VITESS_MIGRATION ... CLEANUP statement #9160

Merged

Conversation

shlomi-noach
Copy link
Contributor

@shlomi-noach shlomi-noach commented Nov 8, 2021

Followup to #9158 (9158 is unmerged yet, this PR extends it)
Tracking: #6926

This PR offers a new statement:

alter vitess_migration '9748c3b7_7fdb_11eb_ac2c_f875a4d24e90' cleanup

Normally, when a migration is complete, it takes 24h (or however -retain_online_ddl_tables is configured) until the artifacts are garbage collected and removed.

With this new SQL statement we can tell vitess that a migration's artifacts are good to garbage collect now. The statement itself does not clean the artifacts, and returns immediately. It merely sets a small (negative) value to retain_artifacts_seconds (see #9158) so that the next poll interval, typically a matter of seconds, can pick up on those artifacts and begin the garbage collection process.

Notice that once a migration's artifacts are cleaned up, the migration may not be eligible for revert.

Checklist

  • Should this PR be backported?
  • Tests were added or are not required
  • Documentation was added or is not required

Docs are required.

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
@shlomi-noach shlomi-noach added Type: Enhancement Logical improvement (somewhere between a bug and feature) Component: Query Serving release notes (needs details) This PR needs to be listed in the release notes in a dedicated section (deprecation notice, etc...) labels Nov 8, 2021
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
@shlomi-noach shlomi-noach marked this pull request as ready for review November 9, 2021 07:06
@shlomi-noach
Copy link
Contributor Author

ready for review

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.

Parser changes look good to me! 🏆

@shlomi-noach shlomi-noach requested a review from deepthi November 10, 2021 09:43
@shlomi-noach
Copy link
Contributor Author

/cc @deepthi @sougou for review

@shlomi-noach shlomi-noach requested a review from sougou November 15, 2021 13:11
Copy link
Member

@deepthi deepthi left a comment

Choose a reason for hiding this comment

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

Non-parser changes look good to me.
Minor suggestion on release notes, otherwise 👍

```sql
alter vitess_migration '9748c3b7_7fdb_11eb_ac2c_f875a4d24e90' cleanup
```
This query tells Vitess that a migration's artifacts are good to be cleaned up asap. This allows Vitess to free disk resources sooner. As reminder, once a migration's artifacts are cleaned up, the migration is no
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This query tells Vitess that a migration's artifacts are good to be cleaned up asap. This allows Vitess to free disk resources sooner. As reminder, once a migration's artifacts are cleaned up, the migration is no
This query tells Vitess that a migration's artifacts are good to be cleaned up asap. This allows Vitess to free disk resources sooner. As a reminder, once a migration's artifacts are cleaned up, the migration is no

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
@shlomi-noach
Copy link
Contributor Author

I resolved conflicts with main, which merged in the new linter checks. This PR therefore also modifies (minorly) some unrelated code that has to pass new linting.

@shlomi-noach shlomi-noach merged commit 3d33f4a into vitessio:main Nov 17, 2021
@shlomi-noach shlomi-noach deleted the alter-vitess-migration-cleanup branch November 17, 2021 08:46
@shlomi-noach
Copy link
Contributor Author

Documentation PR: vitessio/website#886

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Query Serving release notes (needs details) This PR needs to be listed in the release notes in a dedicated section (deprecation notice, etc...) Type: Enhancement Logical improvement (somewhere between a bug and feature)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants