Disable foreign keys for internal updates

[masnax]

We can't be sure which tables specified by external users of microcluster reference internal tables. So just broadly turn off foreign keys while applying internal schema updates, and turn it back on afterward.

[masnax]

This one should get merged ASAP as the bug may result in data loss if anyone is using the latest microcluster.

[masnax]

Because PRAGMA foreign_keys=OFF has no effect if invoked in a transaction, we have to split up the transaction into multiple parts. I've now updated this PR to split up the transaction where we run schema updates into 4 parts.

• The first transaction checks if the schemas table exists.

  • If the schema table does exist, then we check if we need to run the special v1 update that modifies the schema table so that the queries we make to it as part of the schema update process do not fail.
  • If we need to run the v1 update, then we turn off foreign keys before the next transaction so that external references to the internal_cluster_members table are not wiped.

• The second transaction applies the special v1 update, and fetches the maximum internal and external schema versions from the schemas table.

  • If we detect that some nodes are behind with the schema.ErrGracefulAbort, we return early.
  • If we need to return early, we re-enable foreign keys.
  • If we are not returning early, and our max internal schema version implies that we have updates to apply, then we turn off foreign keys outside the transaction if we didn't turn them off after the first transaction.

• The third transaction applies internal schema updates.

  • After all the internal schema updates are committed, we re-enable foreign keys before proceeding to external schema updates.

• The final transaction applies external schema updates.

[tomponline]

Please can we chat about this next week at the sprint. This is starting to sound rather complicated and i dont understand why it has gotten to be this way. Thanks

[tomponline]

Also LXD has been known to have to do things like this occasionally in patches, but to my knowledge its not done routinely like this.

[masnax]

Also LXD has been known to have to do things like this occasionally in patches, but to my knowledge its not done routinely like this.

LXD is quite a different situation than microcluster because LXD is in full control of its schema. If we need to make a change, we know and can manage any relationships between the tables. In microcluster this isn't the case. We don't know what the upstream project's supplied schema looks like so we must be very careful when we make changes to the internal schema, and we must consider that alterations to the internal schema can affect relations to external tables that we dont know about.

It was intended for microcluster to supply its internal tables as a source of truth for the cluster state to upstream projects. MicroCeph for example needs to associate OSDs to the cluster member that supplied the disks, and does so through a reference to the internal_cluster_members table.

So we are left with non-trivial options. I can think of these:

• We can leave things as they are and just fix the existing schema updates to use ADD COLUMN instead of replacing the whole table and remember to never drop any tables, rows, or columns in any schema update in the future.

• We disable foreign keys on a case by case basis. I think we will end up doing this more often than not because any alteration to an existing table will be at risk. We only wouldn't need to disable foreign keys if we are just adding a table, and there's not really many more internal tables that microcluster needs. Since you can't disable foreign keys in a transaction, we would either have to explicitly run every schema update in its own transaction, or we would have to invoke something like this in each schema update call to tx.Exec:

COMMIT TRANSACTION;
PRAGMA foreign_keys=OFF;
BEGIN TRANSACTION;
// body of schema update
COMMIT TRANSACTION;
PRAGMA foreign_keys=ON;
BEGIN TRANSACTION;

• We blanket disable foreign keys for the whole internal schema update process.

• We can reasses the approach of allowing external tables to reference internal ones. This would require a hefty change in all upstream projects as they update their schemas. We would then also need to implement some synching mechanisms so that external tables are updated appropriately when a change is made on the internal side (eg. a cluster member is removed or modified).

[tomponline]

Thanks Max. I think this warrants an architectural discussion around what micro cluster provides to its consumers. Thanks for laying out the landscape of options.

[tomponline]

• If we detect that some nodes are behind with the schema.ErrGracefulAbort, we return early.

Is this effectively the same as patchDBNodesAutoInc in LXD? Albeit it sounds more complex in the microcluster case somehow.

[masnax]

• If we detect that some nodes are behind with the schema.ErrGracefulAbort, we return early.

Is this effectively the same as patchDBNodesAutoInc in LXD? Albeit it sounds more complex in the microcluster case somehow.

I'm a bit confused by the line you quoted because I don't see the connection between that function and the error that makes us abort updates.

The purpose of the function seems similar enough to me if I understand it correctly, the key difference I see is that needing foreign keys to be disabled is a special case for LXD's schema extensions but they are really the only type of schema updates I forsee us needing in microcluster.

As for the complexity of needing to split the transaction up into 4 parts, which I'm assuming is what you meant with the line you quoted, the reason for that is just to accommodate the #94 schema update fix and the two sets of schema updates that are unique to microcluster.

In LXD's case, the process looks like this:

• We check for existence of the schema table.
• Then do some setup like creating that table if we need to, and checking if we need to apply any updates.
• Finally we apply the updates or return early.
• We also have patches that run at different stages of the daemon start proces.

Microcluster is similar:

• Check for existence of the schema table.
• Then we run the update from #94 to fix the broken default schema table. We could say this is analogous to a one-and-done patch without a whole mechanism behind it.
• Then comes the setup like in LXD
• Apply internal updates if needed
• Apply external updates if needed.

We need to disable foreign keys in two cases: Bullet points 2 and 4, hence needing to break the transaction up into parts.

This approach makes us only disable foreign keys if we are absolutely sure we are going to modify the internal schema. If we want, we can be more relaxed and just disable foreign keys blindly up until we reach the point where we want to apply external updates, at which point we can enable foreign keys first. That would get rid of all the if weReallyNeedTo type statements, and we would only need 2 transactions (1 for bullet points 1-4, and 1 for bullet point 5).

[masnax]

@roosterfish @tomponline

I've now tested both references with FOREIGN KEY, as well as VIEWs will still exist with a dropped or renamed column or table, and can be updated to reference a new column.

For this to work, we need to set PRAGMA foreign_keys=OFF and PRAGMA legacy_alter_table=ON before each transaction in which we run any changes to the internal schema.

[roosterfish]

For this to work, we need to set PRAGMA foreign_keys=OFF and PRAGMA legacy_alter_table=ON before each transaction in which we run any changes to the internal schema.

Cool, so this means we can proceed with the plan which allows a downstream importer of MicroCluster to supply an external DB patch which will then align the external tables to the internal ones which have been intentionally "broken" by the internal schema update?