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

schemachanger: enable reverting failed schema change jobs #61466

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

thoszhang
Copy link
Contributor

First commit is #61042.

This PR prevents new-style schema changes from running concurrently with
any other schema changes, in two ways:

1. If there are mutations in progress (from either new or old schema
   changes) when attempting to plan a new-style schema change, we wait
   and poll until there are no mutations, and then restart the
   transaction.
2. If we try to write an old-style schema change job while there is a
   new-style schema change on the table, which is detected via a new
   field on the table descriptor for the new-style schema change job ID,
   an error is returned. This effectively prevents all schema changes,
   even the ones without mutations.

Most of this commit consists of testing. Testing knobs for the new
schema changer are introduced. We also now accumulate the statements
involved in the schema change, as strings, in the schema changer state
in `extraTxnState`. The executor now takes an argument to inject more
relevant state into the testing knobs, including the aforementioned
statements.

Release justification: Non-production code change (the new schema
changer is disabled for 21.1)

Release note: None
Release justification: Non-production code change (the schema changer is
not enabled by default for 21.1).

Release note: None
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@thoszhang thoszhang requested review from ajwerner and a team March 4, 2021 14:57
@thoszhang
Copy link
Contributor Author

I think the code changes are pretty simple, but the main thing this is missing is testing. It'd be nice to have some data-driven/declarative planning tests for the stages generated when reverting a schema change, with DDL statements as the input. But it's not clear what the best way would be to specify in the test input the exact stage at which we'd want to stop and revert. Maybe we could specify the number of stages to run.

@postamar postamar removed the request for review from a team August 17, 2021 12:50
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.

2 participants