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

catalog: Key migration shard by binary version #31210

Conversation

jkosh44
Copy link
Contributor

@jkosh44 jkosh44 commented Jan 27, 2025

The builtin migration shard is a persist shard used during version
upgrades. It uses the environment's deploy generation as part of the
key for the values. The assumption was that two environments with the
same deploy generation would always have the same binary version. This
assumption would allow all migration steps of two environments with the
same deploy generation to be idempotent.

This assumption was not correct. Two environments with different binary
version can use the same deploy generation as long as one environment
never fully completed a deployment. This is especially bad because the
migration shard is written to and read from in read-only mode, before
a deployment is complete.

This commit updates the key of the builtin migration shard to
explicitly use the binary version of environmentd so that the
migration steps are idempotent.

Fixes #MaterializeInc/database-issues/issues/8917

Motivation

This PR fixes a previously unreported bug.

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

@jkosh44 jkosh44 force-pushed the stop-using-deploy-generation-builtin-migration-shard branch from 1758729 to fb521ad Compare January 27, 2025 21:53
The builtin migration shard is a persist shard used during version
upgrades. It uses the environment's deploy generation as part of the
key for the values. The assumption was that two environments with the
same deploy generation would always have the same binary version. This
assumption would allow all migration steps of two environments with the
same deploy generation to be idempotent.

This assumption was not correct. Two environments with different binary
version can use the same deploy generation as long as one environment
never fully completed a deployment. This is especially bad because the
migration shard is written to and read from in read-only mode, before
a deployment is complete.

This commit updates the key of the builtin migration shard to
explicitly use the binary version of environmentd so that the
migration steps are idempotent.

Fixes #MaterializeInc/database-issues/issues/8917
@jkosh44 jkosh44 force-pushed the stop-using-deploy-generation-builtin-migration-shard branch from fb521ad to 7a635c9 Compare January 27, 2025 22:22
@jkosh44 jkosh44 marked this pull request as ready for review January 27, 2025 22:50
@jkosh44 jkosh44 requested a review from a team as a code owner January 27, 2025 22:50
@jkosh44 jkosh44 requested a review from ParkMyCar January 27, 2025 22:50
@jkosh44 jkosh44 merged commit e91678f into MaterializeInc:main Jan 28, 2025
242 checks passed
@jkosh44 jkosh44 deleted the stop-using-deploy-generation-builtin-migration-shard branch January 28, 2025 21:04
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