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

restore: fix full cluster restore after public schema migration #71479

Closed
RichardJCai opened this issue Oct 12, 2021 · 5 comments · Fixed by #71890
Closed

restore: fix full cluster restore after public schema migration #71479

RichardJCai opened this issue Oct 12, 2021 · 5 comments · Fixed by #71890
Assignees
Labels
A-disaster-recovery C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@RichardJCai
Copy link
Contributor

RichardJCai commented Oct 12, 2021

As of today (21.2) IDs 52 and onwards are used by objects created by the user.

Once the public schema migration has been completed, new clusters created on CockroachDB versions >=22.1 will use IDs 52, 53 an in order to create two new public schemas for the default created dbs (defaultdb, postgres).
(defaultdb 50, defaultdb public schema 51, postgres, 52, postgres public schema 53)

This creates an issue with full cluster restore as we currently don't support re-keying.

For example, if we backed up a cluster in 21.2 where the user simply created one table t and inserted some rows, t would use ID 52. Restoring this table into a cluster where the public schema migration has been completed will be an issue since 52 will be used by postgres database.

The solution to this would be to drop both defaultdb and postgres perform the restore where we are guaranteed those IDs are not being used by the dbs, and recreate the databases with their public schemas afterwards.

This is similar to how we create the temporary system database for restore above the ID space (above the maximum id being restored).

cc @dt @ajwerner

Epic CRDB-1521

@blathers-crl
Copy link

blathers-crl bot commented Oct 12, 2021

Hi @RichardJCai, I've guessed the C-ategory of your issue and suitably labeled it. Please re-label if inaccurate.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@blathers-crl blathers-crl bot added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Oct 12, 2021
@RichardJCai
Copy link
Contributor Author

Should I put this under @cockroachdb/bulk-io or @cockroachdb/sql-experience since I'll probably be implementing this?

@shermanCRL
Copy link
Contributor

I think it needs to start with on the SQL team. cc @adityamaru

@shermanCRL shermanCRL added the T-sql-schema-deprecated Use T-sql-foundations instead label Oct 12, 2021
@adityamaru
Copy link
Contributor

Sorry I couldn't join the meeting so just catching up. My first reaction was ugh more static IDs 😄 but I guess there is more context here. Dropping the databases before a cluster restore does simplify stuff a lot. I'll catch up on the notes!

@dt
Copy link
Member

dt commented Oct 12, 2021

@adityamaru heh, sorry, we didn't keep detailed notes, but essentially plan is to 1) first make restoring into a cluster with realized public schema descriptors work, and then 2) make restoring in a world with dynamically assigned system and default IDs work.

Doing 1) requires a) materializing new public schema descriptors if the backup is missing them (e.g. backed up pre-migration) and then re-parenting things, of course, but also, first b) moving any existing database or schemas descriptors in the restoring cluster that have ID > min-id-in-backup, to above the last ID in the backup, so as to avoid conflicting with IDs in the backup, so we're not forced to re-key to avoid them at their current IDs. We quibbled on whether or not to do this, if we're headed to a world in 2) where we might need to rekey to avoid system tables in the restoring cluster that have been created in our backup's ID space, but we decided it was worth doing it anyway, since it both unblocked the public schema thing now, without waiting for rekeying, and also because even if we could rekey, it is cheaper to move a descriptor to a new ID than a whole table, so we'd probably keep this around even when we add dynamic system table ids and rekeys.

@RichardJCai RichardJCai self-assigned this Oct 13, 2021
@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Oct 13, 2021
@exalate-issue-sync exalate-issue-sync bot removed T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) T-disaster-recovery labels Nov 4, 2021
@craig craig bot closed this as completed in 2de17e7 Nov 10, 2021
@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-disaster-recovery C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
No open projects
Archived in project
4 participants