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

upgrade/upgrades: allow CreatedAtNanos to be set when validating migration #87462

Merged

Conversation

ajwerner
Copy link
Contributor

@ajwerner ajwerner commented Sep 6, 2022

Schema change upgrade migrations to system tables are made idempotent by checking that the descriptor reaches some expected state. In order to ensure that it is in that expected state, some volatile fields need to be masked. We forgot to mask CreatedAtNanos. We also lost the testing which came with these helper functions we use.

The vast majority of this PR is reviving testing from #66889.

Fixes #85228.

Release justification: Import bug fix for backport

Release note (bug fix): Some upgrade migrations perform schema changes on system tables. Those upgrades which added indexes could, in some cases, get caught retrying because they failed to detect that the migration had already occurred due to the existence of a populated field. When that happens, the finalization of the new version can hang indefinitely and require manual intervention. This bug has been fixed.

@ajwerner ajwerner requested a review from a team September 6, 2022 22:11
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@ajwerner ajwerner force-pushed the ajwerner/fix-upgrade-index-creation branch 2 times, most recently from 329dad0 to 5beedb7 Compare September 7, 2022 20:59
…ation

Schema change upgrade migrations to system tables are made idempotent by
checking that the descriptor reaches some expected state. In order to ensure
that it is in that expected state, some volatile fields need to be masked.
We forgot to mask CreatedAtNanos. We also lost the testing which came with
these helper functions we use.

The vast majority of this PR is reviving testing from cockroachdb#66889.

Fixes cockroachdb#85228.

Release justification: Import bug fix for backport

Release note (bug fix): Some upgrade migrations perform schema changes on
system tables. Those upgrades which added indexes could, in some cases, get
caught retrying because they failed to detect that the migration had already
occurred due to the existence of a populated field. When that happens, the
finalization of the new version can hang indefinitely and require manual
intervention. This bug has been fixed.
@ajwerner ajwerner force-pushed the ajwerner/fix-upgrade-index-creation branch from 5beedb7 to 52eda07 Compare September 7, 2022 22:14
@ajwerner
Copy link
Contributor Author

ajwerner commented Sep 8, 2022

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented Sep 8, 2022

Build succeeded:

@blathers-crl
Copy link

blathers-crl bot commented Sep 8, 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 52eda07 to blathers/backport-release-22.1-87462: 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.1.x 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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

upgrades: index validation should ignore CreatedAtNanos field
3 participants