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.

Sign up for GitHub

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

Jump to bottom

upgrade: adding is_draining to sql_instances fails on MR system database #135736

Closed
fqazi opened this issue Nov 19, 2024 · 1 comment · Fixed by #135737
Closed

upgrade: adding is_draining to sql_instances fails on MR system database #135736

fqazi opened this issue Nov 19, 2024 · 1 comment · Fixed by #135737
Assignees
@fqazi
Labels
branch-master Failures and bugs on the master branch. branch-release-24.3 Used to mark GA and release blockers, technical advisories, and bugs for 24.3 branch-release-24.3.0-rc C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@fqazi
Copy link
Collaborator

fqazi commented Nov 19, 2024 •
edited by cockroach-jira-scripts
Loading

When upgrading from 24.2 to 24.3 with a multi-region system database the upgrade the can fail with the following error:

 running execution encountered retriable error: non-cancelable: running migration for 24.2-upgrading-to-24.3-step-014: unable to replace system descriptor for system.‹sql_instances› ...  invalid locality config: database ‹system› is multi-region enabled, but table ‹sql_instances› has no locality set

This happens because the existing upgrade attempts to copy the bootstrap descriptor directly. While this is fine for single region system databases. This is approach does not work for multi-region system databases. To address this we should just use an alter command for the upgrade.

Jira issue: CRDB-44694
@fqazi fqazi added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. branch-master Failures and bugs on the master branch. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) branch-release-24.3 Used to mark GA and release blockers, technical advisories, and bugs for 24.3 labels Nov 19, 2024
@fqazi fqazi self-assigned this Nov 19, 2024
fqazi added a commit to fqazi/cockroach that referenced this issue Nov 19, 2024
@fqazi
upgrade: adding is_draining to system.sql_instance can fail
50584f1 
Previously, the logic to add the is_draining column to the
system.sql_instance descriptor would copy the bootstrap descriptor
directly. While this works fine for non-multiregion system databases, this
approach breaks for multi-region system databases. This is because
bootstrap descriptors do not have multi-region modifications applied on
top. To address this, this change modifies the upgrade to use ALTER
TABLE ADD COLUMN.

Fixes: cockroachdb#135736

Release note: None
@fqazi fqazi mentioned this issue Nov 19, 2024
Merged
fqazi added a commit to fqazi/cockroach that referenced this issue Nov 19, 2024
@fqazi
upgrade: adding is_draining to system.sql_instance can fail
c36ef72 
Previously, the logic to add the is_draining column to the
system.sql_instance descriptor would copy the bootstrap descriptor
directly. While this works fine for non-multiregion system databases, this
approach breaks for multi-region system databases. This is because
bootstrap descriptors do not have multi-region modifications applied on
top. To address this, this change modifies the upgrade to use ALTER
TABLE ADD COLUMN.

Fixes: cockroachdb#135736

Release note: None
fqazi added a commit to fqazi/cockroach that referenced this issue Nov 19, 2024
@fqazi
upgrade: adding is_draining to system.sql_instance can fail
ae3184d 
Previously, the logic to add the is_draining column to the
system.sql_instance descriptor would copy the bootstrap descriptor
directly. While this works fine for non-multiregion system databases, this
approach breaks for multi-region system databases. This is because
bootstrap descriptors do not have multi-region modifications applied on
top. To address this, this change modifies the upgrade to use ALTER
TABLE ADD COLUMN.

Fixes: cockroachdb#135736

Release note: None
craig bot pushed a commit that referenced this issue Nov 20, 2024
@fqazi
Merge #135737
873e633 
135737: upgrade: adding is_draining to system.sql_instance can fail r=fqazi a=fqazi

Previously, the logic to add the is_draining column to the system.sql_instance descriptor would copy the bootstrap descriptor directly. While this works fine for non-multiregion system databases, this approach breaks for multi-region system databases. This is because bootstrap descriptors do not have multi-region modifications applied on top. To address this, this change modifies the upgrade to use ALTER TABLE ADD COLUMN.

Fixes: #135736

Release note: None

Co-authored-by: Faizan Qazi <[email protected]>
@craig craig bot closed this as completed in 0509190 Nov 20, 2024
@blathers-crl blathers[crl]
Copy link

blathers-crl bot commented Nov 20, 2024

Based on the specified backports for linked PR #135737, I applied the following new label(s) to this issue: branch-release-24.3.0-rc. Please adjust the labels as needed to match the branches actually affected by this issue, including adding any known older branches.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

fqazi added a commit to fqazi/cockroach that referenced this issue Nov 20, 2024
@fqazi
upgrade: adding is_draining to system.sql_instance can fail
284e9cc 
Previously, the logic to add the is_draining column to the
system.sql_instance descriptor would copy the bootstrap descriptor
directly. While this works fine for non-multiregion system databases, this
approach breaks for multi-region system databases. This is because
bootstrap descriptors do not have multi-region modifications applied on
top. To address this, this change modifies the upgrade to use ALTER
TABLE ADD COLUMN.

Fixes: cockroachdb#135736

Release note: None
@fqazi fqazi mentioned this issue Nov 20, 2024
Merged
fqazi added a commit to fqazi/cockroach that referenced this issue Nov 20, 2024
@fqazi
upgrade: adding is_draining to system.sql_instance can fail
15419c6 
Previously, the logic to add the is_draining column to the
system.sql_instance descriptor would copy the bootstrap descriptor
directly. While this works fine for non-multiregion system databases, this
approach breaks for multi-region system databases. This is because
bootstrap descriptors do not have multi-region modifications applied on
top. To address this, this change modifies the upgrade to use ALTER
TABLE ADD COLUMN.

Fixes: cockroachdb#135736

Release note: None
@fqazi fqazi mentioned this issue Nov 20, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fqazi fqazi

Labels
branch-master Failures and bugs on the master branch. branch-release-24.3 Used to mark GA and release blockers, technical advisories, and bugs for 24.3 branch-release-24.3.0-rc C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

upgrade: adding is_draining to system.sql_instance can fail fqazi/cockroach
1 participant
@fqazi