Add notes on fixing broken schema updates

[smklein]

Addresses feedback on #6436

[smklein]

This section was moved lower, as it's more of an "FYI" than guidance.

[andrewjstone]

Thanks @smklein. Love the writeup about convergence.

[Nieuwejaar]

As a practical matter, can we ever be sure that all customer data has migrated this far? I guess if we established a requirement that the cluster boot/run at schema level N before being allowed to migrate to N+M, it might be possible. Which is a long way of saying that "If all data..." might be more apt than "Once all data...".

[smklein]

Yeah, I was intentionally very handwavey with "how do you be certain the migration has completed", because it's very dependent on your data.

If it's possible to back-fill this data by observing the database, our current schema-updating system acts on a system with no other concurrent queries, so it should be possible to back-fill these values based on other tables and then make the conversion.

If backfilling this data requires reading data outside the database -- maybe we want to add a non-nullable field to inventory? -- then this is much harder. We basically rely on Nexus performing work after some schema updates are applied, but before the "nullability" constraint has been fully dropped. Right now, we don't really have a great way of doing this, and we'll probably need to build more mechanism to make this possible.

[davepacheco]

I proposed more specific language. I think the only realistic choice for the time being is to migrate the data yourself during the update. We've done that a bunch of times.

An alternative might be to ship the update over two releases: one that adds the column and whose Nexus backfills data in the meantime; and one that makes the column required. But this doesn't work unless there's some kind of interlock to ensure that the backfill process has finished before somebody starts the second upgrade.

An alternative is to ship an ad hoc Rust program alongside schema-updater (or a subcommand of that program) that backfills the data and maybe knows to do it between two specific schema versions. This is probably the most promising but let's not do that if we don't have to.

[davepacheco]

Suggested change

	Unfortnately, it's possible that the schema upgrade might NOT fail with that
	Unfortunately, it's possible that the schema upgrade might NOT fail with that

[smklein]

Updated

[davepacheco]

I believe we've done this a few times by being willing to rewrite the data during a migration. Schema version 26 seems like an example, though it's kind of a complicated one. Schema version 54 (mentioned below) is also an example of populating a new column but it did not make the new column NOT NULL after doing it.

What about being more specific here for the easy case and less specific for the general case. Maybe something like:

If a DEFAULT value does not make sense, then you need to implement a multi-step process.

1. Add the column without a NOT NULL constraint.
2. Migrate existing data to a non-null value.
3. Once all data has been migrated to a non-null value, alter the table again to add the NOT NULL constraint.

For the time being, if you can write the data migration in SQL (e.g., using a SQL UPDATE), then you can do this with a single new schema version where the second step is an UPDATE. See schema version 54 (blueprint-add-external-ip-id) for an example of this (though that one did not make the new column NOT NULL -- you'd want to do that in another step). Update the validate_data_migration() test in nexus/tests/integration_tests/schema.rs to add a test for this.

In the future when schema updates happen while the control plane is online, this may not be a tenable path because the operation may take a very long time on large tables.

If you cannot write the data migration in SQL, you would need to figure out a different way to backfill the data before you can apply the step that adds the NOT NULL constraint. This is likely a substantial project.

You don't have to take that edit. But basically, I'm worried the language that's here now isn't giving enough guidance for the common case that you can just put the UPDATE into a migration; but it also makes it sound potentially okay if the data doesn't get migrated promptly.

[smklein]

Updated. FWIW I do still think that the "schema migrations that we can make from reading the DB alone" will still totally be tractable, even with online update (though, that whole area is speculative). But yeah, totally agreed that the backfill outside the DB will be much harder, and require some back-and-forth between "Nexus running normally" and "Nexus updating schemas".

[davepacheco]

Yeah -- that makes me realize that when we are doing online update, then the control plane itself can know not to start the update until the data has been backfilled.

[davepacheco]

I proposed more specific language. I think the only realistic choice for the time being is to migrate the data yourself during the update. We've done that a bunch of times.

An alternative might be to ship the update over two releases: one that adds the column and whose Nexus backfills data in the meantime; and one that makes the column required. But this doesn't work unless there's some kind of interlock to ensure that the backfill process has finished before somebody starts the second upgrade.

An alternative is to ship an ad hoc Rust program alongside schema-updater (or a subcommand of that program) that backfills the data and maybe knows to do it between two specific schema versions. This is probably the most promising but let's not do that if we don't have to.

[davepacheco]

Suggested change

	appended to the list of all updates, rather than re-ordering history.
	appended to the list of all updates, rather than re-ordering history. This schema update will run on systems that deployed both versions of the earlier schema update.
	. Determine whether any of the schema versions after the broken one need to change because they depended on the specific behavior that you changed to _fix_ that version.

You kind of said this later but I feel like it will be missed if it's not explicitly one of the steps we tell people to follow.

[smklein]

Updated

[davepacheco]

I'm a little worried here that systems running versions (S(bad), S(converge)) may still be in inconsistent states...but that may be unavoidable.

[smklein]

Yeah, agreed. Happy to cover this more if we feel like we can, but this whole area is "documenting what to do when some ambiguous bad thing has already happened".

I think our inclination towards "try to prevent the bad thing from happening via tests" (e.g. #6451) should take priority, largely because it feels more tractable to me.