Skip to content

upgrades: skip known userfile-related corruption in desc validation#88878

Merged
craig[bot] merged 1 commit into
cockroachdb:masterfrom
stevendanna:ssd/userfile-mutation-migration
Oct 4, 2022
Merged

upgrades: skip known userfile-related corruption in desc validation#88878
craig[bot] merged 1 commit into
cockroachdb:masterfrom
stevendanna:ssd/userfile-mutation-migration

Conversation

@stevendanna
Copy link
Copy Markdown
Collaborator

In previous releases, userfiles would create broken tables because
of incorrect use of the internal executor. As a result, any user that
has used userfile, would fail the 22.2 upgrade precondition check.

This does 2 things:

  • 1 allows the precondition to pass even in the presence of an invalid
    object as long as that object is very likely the userfile payload table.

  • Adds a new migration that removes the foreign key mutation for userfile
    payload tables.

We determine whether something is a "userfile payload table" by checking
its name, the column names, the column types, the name of the referenced table,
the column names of the referenced table, and the column types of the referenced table.

Release justification: Avoid having a failed upgrade for every user of
userfile.

@stevendanna stevendanna requested a review from a team September 27, 2022 23:45
@stevendanna stevendanna requested a review from a team as a code owner September 27, 2022 23:45
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@stevendanna stevendanna requested review from adityamaru, ajstorm and ajwerner and removed request for ajstorm September 28, 2022 16:36
Copy link
Copy Markdown
Contributor

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

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

:lgtm:

This is good, thanks for doing it!

Reviewed 5 of 5 files at r1, 4 of 8 files at r2.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @adityamaru and @ajwerner)

@stevendanna stevendanna force-pushed the ssd/userfile-mutation-migration branch 4 times, most recently from d1a3c55 to 8659628 Compare October 3, 2022 15:02
@stevendanna stevendanna added branch-release-22.2 Used to mark GA and release blockers, technical advisories, and bugs for 22.2 backport-22.2.x labels Oct 3, 2022
@stevendanna
Copy link
Copy Markdown
Collaborator Author

bors r=ajwerner

@ajwerner
Copy link
Copy Markdown
Contributor

ajwerner commented Oct 3, 2022

Should we add a userfile step to the mixed version roachtests?

@craig
Copy link
Copy Markdown
Contributor

craig Bot commented Oct 3, 2022

Build failed (retrying...):

@craig
Copy link
Copy Markdown
Contributor

craig Bot commented Oct 3, 2022

Build failed (retrying...):

@yuzefovich
Copy link
Copy Markdown
Member

I think there is a merge skew with #88875, and it looks like the batch with that PR has smaller number and should go in sooner, so this will need to be rebased.

bors r-

@craig
Copy link
Copy Markdown
Contributor

craig Bot commented Oct 3, 2022

Canceled.

@stevendanna
Copy link
Copy Markdown
Collaborator Author

@yuzefovich Thanks

@stevendanna stevendanna force-pushed the ssd/userfile-mutation-migration branch from 8659628 to 73dab2d Compare October 4, 2022 00:37
The related job for this FK constraint was never created because of a
userfile error. This migration removes the constraint since it is not
currently needed.

Release justification: Avoid having a failed upgrade for every user of
userfile.

Release note (bug fix): Fixes tables created by userfile storage that
have invalid foreign key constraints.
@stevendanna stevendanna force-pushed the ssd/userfile-mutation-migration branch from 73dab2d to aaaea8b Compare October 4, 2022 02:05
@stevendanna
Copy link
Copy Markdown
Collaborator Author

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig Bot commented Oct 4, 2022

Build failed:

@stevendanna
Copy link
Copy Markdown
Collaborator Author

bors retry

@craig
Copy link
Copy Markdown
Contributor

craig Bot commented Oct 4, 2022

Build succeeded:

@craig craig Bot merged commit fc50f10 into cockroachdb:master Oct 4, 2022
@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Oct 4, 2022

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


error creating merge commit from aaaea8b to blathers/backport-release-22.2-88878: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 22.2.x failed. See errors above.


error creating merge commit from aaaea8b to blathers/backport-release-22.2.0-88878: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 22.2.0 failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch-release-22.2 Used to mark GA and release blockers, technical advisories, and bugs for 22.2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants