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

sql: creating new system tables with sqlmigrations is not possible #59850

Closed
ajwerner opened this issue Feb 5, 2021 · 2 comments
Closed

sql: creating new system tables with sqlmigrations is not possible #59850

ajwerner opened this issue Feb 5, 2021 · 2 comments
Labels
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
SQL Foundations

Comments

@ajwerner
Copy link
Contributor

ajwerner commented Feb 5, 2021 •
edited

Describe the problem

I think we may have very badly shot ourselves in the foot in a way I would not have anticipated. I'm feeling sad about it. We added validation all over the place in 20.2, as we well know. One of the key places where we added this validation was in virtual tables. One interesting thing that happens in validation is here. We check that the system tables have some privileges in some map. Unfortunately, that means that when adding new system tables, in the mixed version state, they will fail validation. This is a tough nut to crack. It means that, at least for 21.1, we can't add new system tables during sqlmigrations. I can work around this for my current use case but this is really unfortunate in general. I'm not sure how best to test this sort of thing.

To Reproduce

Attempt to add a new system table and then in the mixed version state, run SHOW TABLES or something like it.

Expected behavior

We can create new system tables and the mixed-version state will work.

Additional Context

Maybe this isn't actually too terrible. In 21.1 we have long-running migrations which only run after all nodes are on the new version. That means we can safely add tables there. The only really awkward part is the migration to bootstrap the infrastructure for long-running migrations.
@ajwerner ajwerner added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Feb 5, 2021
@ajwerner ajwerner added this to Triage in SQL Foundations via automation Feb 5, 2021
ajwerner added a commit to ajwerner/cockroach that referenced this issue Feb 7, 2021
@ajwerner
sql,systemschema,migration: add system.long_running_migrations, adopt
6c31b48 
This commit adds a table, `system.long_running_migrations`, to store the
completion status of long-running migrations. This table is used to ensure
safety and isolation in the face of concurrent attempts to upgrade the
cluster version.

An important facet of this change is the migration to add this table
to the cluster during upgrades. Given that the implementation of long-running
migrations relies on the table's existence, all versions which are associated
with long-running migrations must follow the version which introduces this
table. This migration needs to utilize the new infrastructure rather than
the `sqlmigrations` infrastructure because of some really bad behavior
introduced in 20.2 (cockroachdb#59850). Given two such versions already existed, they
have been assigned new version keys and their old version keys and values
are now merely placeholders. This provides comptability with the earlier alphas.

Release note: None
ajwerner added a commit to ajwerner/cockroach that referenced this issue Feb 9, 2021
@ajwerner
sql,systemschema,migration: add system.long_running_migrations, adopt
cdd04f8 
This commit adds a table, `system.long_running_migrations`, to store the
completion status of long-running migrations. This table is used to ensure
safety and isolation in the face of concurrent attempts to upgrade the
cluster version.

An important facet of this change is the migration to add this table
to the cluster during upgrades. Given that the implementation of long-running
migrations relies on the table's existence, all versions which are associated
with long-running migrations must follow the version which introduces this
table. This migration needs to utilize the new infrastructure rather than
the `sqlmigrations` infrastructure because of some really bad behavior
introduced in 20.2 (cockroachdb#59850). Given two such versions already existed, they
have been assigned new version keys and their old version keys and values
are now merely placeholders. This provides comptability with the earlier
alphas.

Release note: None
@ajwerner
Copy link
Contributor Author

ajwerner commented Feb 9, 2021

I've reworked #59760 around this. You can create system tables inside of long-running migrations which we now have. I've gotten to the point that I think this should be the last release for the sqlmigrations package. I'm intending to delete it when 21.2 opens unless somebody can convince me of a reason for its continued existence.

@thoszhang thoszhang moved this from Triage to Backlog in SQL Foundations Feb 9, 2021
ajwerner added a commit to ajwerner/cockroach that referenced this issue Feb 9, 2021
@ajwerner
sql,systemschema,migration: add system.long_running_migrations, adopt
c7c5d1a 
This commit adds a table, `system.long_running_migrations`, to store the
completion status of long-running migrations. This table is used to ensure
safety and isolation in the face of concurrent attempts to upgrade the
cluster version.

An important facet of this change is the migration to add this table
to the cluster during upgrades. Given that the implementation of long-running
migrations relies on the table's existence, all versions which are associated
with long-running migrations must follow the version which introduces this
table. This migration needs to utilize the new infrastructure rather than
the `sqlmigrations` infrastructure because of some really bad behavior
introduced in 20.2 (cockroachdb#59850). Given two such versions already existed, they
have been assigned new version keys and their old version keys and values
are now merely placeholders. This provides comptability with the earlier
alphas.

Release note: None
ajwerner added a commit to ajwerner/cockroach that referenced this issue Feb 10, 2021
@ajwerner
sql,systemschema,migration: add system.migrations, adopt
c17795f 
This commit adds a table, `system.migrations`, to store the completion
status of long-running migrations. This table is used to ensure
safety and isolation in the face of concurrent attempts to upgrade the
cluster version.

An important facet of this change is the migration to add this table
to the cluster during upgrades. Given that the implementation of long-running
migrations relies on the table's existence, all versions which are associated
with long-running migrations must follow the version which introduces this
table. This migration needs to utilize the new infrastructure rather than
the `sqlmigrations` infrastructure because of some really bad behavior
introduced in 20.2 (cockroachdb#59850). Given two such versions already existed, they
have been assigned new version keys and their old version keys and values
are now merely placeholders. This provides comptability with the earlier
alphas.

Release note: None
@ajwerner ajwerner changed the title sql: creating new system tables with sqlmigrations is no possible sql: creating new system tables with sqlmigrations is not possible Feb 16, 2021
@jlinder jlinder added the T-sql-schema-deprecated Use T-sql-foundations instead label Jun 16, 2021
@postamar
Copy link
Contributor

It seems like this should be closed.

SQL Foundations automation moved this from Backlog to Closed Jul 29, 2021
@ajwerner ajwerner mentioned this issue Jul 29, 2021
Open
@exalate-issue-sync exalate-issue-sync bot added T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) and removed T-sql-schema-deprecated Use T-sql-foundations instead labels May 10, 2023
@blathers-crl blathers-crl bot added this to Triage in SQL Foundations May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
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
SQL Foundations
  
Done [after migration]
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jlinder @ajwerner @postamar