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

release-20.1: sql: in schema changer, wait for leases of referenced descriptors to update #55375

Merged
merged 1 commit into from
Oct 9, 2020

Conversation

ajwerner
Copy link
Contributor

@ajwerner ajwerner commented Oct 9, 2020

Backport 1/1 commits from #54434.

/cc @cockroachdb/release


In 20.1 we moved the schema changer over to utilize the jobs infrastructure.
Prior to that change, after committing a sql transaction we'd kick off the
schema changer to run for the relevant mutations and then we'd wait for all
modified descriptors to have just one version (after the schema change). In
20.1 we decided for descriptors referenced in schema changes to also create
schema change jobs that have an InvalidMutationID which leads to the job
merely waiting for one version for that descriptor. Unfortunately, that
solution doesn't generally work. In particular, it doesn't work in cases
where we are add an fk constraint that we need to mark validated at the
end and it doesn't work in cases where we want to remove a type back
reference after dropping a column or when adding a type reference.

Realistically, when the schema change job ends up touching multiple
descriptors, we need to make sure we wait for the leases to update
for all of them.

You might think that this could lead directly to eliminating the
InvalidMutationID schema change jobs, but I'm not convinced that
that always follows and so I'm leaving that for later.

It turns out that I don't think the type portions of this represent
correctness problems as the type references just used for type DDLs but
I do think for tables it might be worse. At the very least it might mean
that query plans a tad longer to reflect fk relationships.

Fixes #52539.
Fixes #40200.

Release note (bug fix): Fixed a bug which allowed statements after a
schema change to fail to observe side-effects of that change on referenced
tables.

…update

In 20.1 we moved the schema changer over to utilize the jobs infrastructure.
Prior to that change, after committing a sql transaction we'd kick off the
schema changer to run for the relevant mutations and then we'd wait for all
modified descriptors to have just one version (after the schema change). In
20.1 we decided for descriptors referenced in schema changes to also create
schema change jobs that have an InvalidMutationID which leads to the job
merely waiting for one version for that descriptor. Unfortunately, that
solution doesn't generally work. In particular, it doesn't work in cases
where we are add an fk constraint that we need to mark validated at the
end and it doesn't work in cases where we want to remove a type back
reference after dropping a column or when adding a type reference.

Realistically, when the schema change job ends up touching multiple
descriptors, we need to make sure we wait for the leases to update
for all of them.

You might think that this could lead directly to eliminating the
InvalidMutationID schema change jobs, but I'm not convinced that
that always follows and so I'm leaving that for later.

It turns out that I don't think the type portions of this represent
correctness problems as the type references just used for type DDLs but
I do think for tables it might be worse. At the very least it might mean
that query plans a tad longer to reflect fk relationships.

Fixes cockroachdb#52539.
Fixes cockroachdb#40200.

Release note (bug fix): Fixed a bug which allowed statements after a
schema change to fail to observe side-effects of that change on referenced
tables.
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@thoszhang thoszhang left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

@ajwerner ajwerner merged commit 61cd549 into cockroachdb:release-20.1 Oct 9, 2020
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.

None yet

3 participants