sql: upgrade 1.1.5 -> 2.0.2 failed to run migration

[kmcrawford]

I upgraded to a dev cluster from 2.0.2 from 1.1.5. The upgrade didn't complete due to the following error:
failed to run migration "upgrade table descs to interleaved format version": parentID 50 does not exist

I was able to rollback to 1.1.5 and can execute queries successfully. However, if I connect to the cluster using the cockroach client and set my database, then run show tables; I get pq: no database with ID 50 found. I spoke to a developer this morning if he has seen anything strange with this instance and he told me things have been strange when he was updating a table and the internet when down before it could complete. I'm not sure if this is what caused this, but I am unable to upgrade the cluster.

@benesch

[tbg]

@knz could you triage?

[knz]

This can happen if a table descriptor lands in an invalid state, e.g. via a schema change.
In general the migrations should be tolerant of invalid table states.

[knz]

@kmcrawford are you able to determine which specific table is causing the problem?

[kmcrawford]

@knz I am not. I do see this in the console that is strange.

[knz]

Yes this is helpful. thanks

[knz]

@mjibson @vivekmenezes can you have a brief look, then we'll talk together to determine what is the best way to move forward (we need both to ensure that failed migrations don't leave bad descriptors around, and also make migrations more tolerant of bad descriptors).

Perhaps an opportunity to revive SCRUB too.

[benesch]

also make migrations more tolerant of bad descriptors

This scares me. The migration in question needs to ensure that every table has been upgraded to the latest format version. If some table descriptors fail to validate how can we be sure that the cluster is in a consistent state? In fact I think this is working as designed: the user is forced to stay on a version that is guaranteed to be able to handle the pre-migration state of the table descriptors.

Obviously it's a huge failing that we've managed to write a bad table descriptor—apologies, @kmcrawford!—but I'd prefer that we build tooling to repair the busted table descriptor rather than attempting to make the system tolerant of bad table descriptors.

[benesch]

@kmcrawford could you run the following query and share the output?

SELECT * FROM crdb_internal.tables;

Unlike SHOW TABLES I believe crdb_internal.tables is tolerant of a missing parent database. You should see (at least) one row with a parent_id of 50. That's the invalid descriptor and it should give us some insight into what's gone wrong. In particular, if it's the delivery.overrides table that's showing up on the jobs page, it's very likely a bug in schema changes.

[vivekmenezes]

can't imagine how this can happen but there is one case where this can happen. When you run DROP DATABASE, the database gets dropped and the tables linger around in the DROP state until the underlying data gets garbage collected. Such tables do not have parentID. And we do upgrade DROP tables during migration

[kmcrawford]

@benesch You are correct, and here is the infamous '50'

[knz]

@benesch I think that a parent-less table should be considered a valid state for a descriptor. As Vivek states there are several known-good situations where this can happen. I suggest we relax the descriptor validation for this specific case.

[vivekmenezes]

So on further looking at this, this problem is a combination of three things.

1. DROP database takes a long time to GC the underlying tables in the 1.1 release . Remember the implementation doesn't use ClearRange(). Therefore a large database dropped weeks earlier might still be around.
2. DROP tables being GC-ed do not show up in the jobs table.
3. The migration path is the only path that can modify a dropped table. After modifying the dropped table it calls validate on it which will fail.

I think the solution here is to not call validate on an upgrade to a dropped table.

[knz]

@vivekmenezes please advise what is your preference.

[vivekmenezes]

@knz I think the minimum we can do here is to not do the descriptor validation for the parent id so that these headless descriptors do not prevent an upgrade. This will at lease unblock the upgrade process. Thoughts?

[knz]

I would recommend running the validation but with a boolean to accept a parent-less table.

[benesch]

I'm not sure that #26543 would help: won't DROP TABLE [50] try to validate the descriptor before it drops it?

There is a third option:

DELETE FROM system.descriptor WHERE id = 50;
DELETE FROM system.namespace WHERE id = 50;

[vivekmenezes]

I'm going to be working on this issue tomorrow.

[vivekmenezes]

The upgrade to the next 2.0 release 2.0.4 (due mid July) should succeed.

[vivekmenezes]

@kmcrawford release 2.0.4 is out

[vivekmenezes]

We were not able to reproduce the dangling database reference for the table. Closing until it is reproducible.