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

[db-manager] Disable implicit transactions for schema migration on CRDB v.22+ #1079

Merged
merged 2 commits into from
Aug 27, 2024

Conversation

barroco
Copy link
Contributor

@barroco barroco commented Aug 23, 2024

While upgrading cockroach db to v24.1.3, RID migrations started to fail on this query from v22.2.0 with the
error Failed to execute defaultdb migration step build/db_schemas/rid/upto-v2.0.0-add_inverted_indices.sql: ERROR: column "cells" is being backfilled (SQLSTATE 42P10).

As documented on the release notes,
the backward incompatible change leading to the error is:

Changed the default value of the enable_implicit_transaction_for_batch_statements to true. 
This means that a batch of statements sent in one string separated by semicolons is treated as an implicit transaction. #76834

After review, no stores in scd or rid packages use batch statements.
There is only one exception, which is used for testing and which should not be impacted.
Therefore, only migration scripts are affected.

This PR ensures no implicit transaction is created for version 22.2 and above by disabling the enable_implicit_transaction_for_batch_statements variable at the beginning of the sql execution for migrations.

This change has been tested locally with version 24.1.3 to validate future usage.

@barroco barroco changed the title [db_schema] Adjust schema migration to support v.22+ of cockroachdb [db_schema] Disable implicit transaction for schema migration on CRDB v.22+ Aug 25, 2024
@barroco barroco changed the title [db_schema] Disable implicit transaction for schema migration on CRDB v.22+ [db-manager] Disable implicit transactions for schema migration on CRDB v.22+ Aug 25, 2024
@barroco barroco marked this pull request as ready for review August 25, 2024 20:34
@barroco barroco merged commit 06a9756 into interuss:master Aug 27, 2024
6 checks passed
barroco added a commit that referenced this pull request Aug 30, 2024
As requested in #1070 in order to properly support indexes used by the DSS, this PR upgrades CockroachDB to version 24.1.3.
A backward incompatibility impacting migrations has been addressed in #1079.
Depends on #1077, #1076 and #1080 to properly upgrade the version.

This PR includes:

Upgrade of all CRDB image references from 21.2.7 to 24.1.3.
Migration notes and references to CRDB documentation.
Helm: Instructions on how to adjust CRDB Helm deployment instructions to map to our Helm chart practices
Tanka: Manual upgrade
Kubernetes steps number have been changed to 1. instead of incremented numbers to reflect editing practices of other documents.
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