sql: TRUNCATE and ongoing index builds are broken

[ajwerner]

Describe the problem

In 20.2 we changed the implementation of TRUNCATE to create all new indexes instead of creating a new table (#52235). This was wonderful in that it made table IDs stable. Unfortunately, it appears that that PR had some severe logic bugs in the face of concurrent index builds.

The logic has two bugs around

cockroach/pkg/sql/truncate.go

Lines 221 to 227 in d145e9f

	// Resolve all outstanding mutations. Make all new schema elements
	// public because the table is empty and doesn't need to be backfilled.
	for _, m := range tableDesc.Mutations {
	if err := tableDesc.MakeMutationComplete(m); err != nil {
	return err
	}
	}

The first is that it should make the in-progress indexes public prior to calculating the re-mapping. Secondly, it needs to deal with the ongoing jobs.

To Reproduce

Create an index backfill which takes a long time and then truncate the table. The index will become public but this index will contain data from prior to the truncate. The job will also remain in the queue which will prevent subsequent schema changes.

Expected behavior

The index is truncated and made public.

Additional context

This affects all current 20.2 versions and 21.1.

[ajwerner]

@fqazi any chance I can enlist you on this? I think the fix is small and the test should be straightforward. This is higher priority than anything else you're currently working on and I believe you have sufficient context.

[fqazi]

Sure I'll take it.

[ajwerner]

I took a stab at this #62944.

[ajwerner]

Alright, after some thinking about this implementation, there's still a lot of scary edge cases. For now the thinking is that we're going to disallow this hazardous behavior. It's modestly unfortunate as there are some cases which would have allowed you to quickly finish long-running schema changes that take a long time to cancel but getting this right in all of the cases seems too hard. Instead we'll allowlist the cases we can convince ourselves are safe and disallow the rest.

[ajwerner]

You can detect this situation in a cluster by running:

  WITH descs AS (
				SELECT d
				  FROM (
						SELECT crdb_internal.pb_to_json(
								'cockroach.sql.sqlbase.Descriptor',
								descriptor,
								false
						       )->'table' AS d
						  FROM system.descriptor
				       )
				 WHERE d IS NOT NULL
             ),
       bad_descs AS (
					SELECT d, idx
					  FROM (
							SELECT d,
							       d->'primaryIndex' AS pi,
							       json_array_elements(d->'indexes') AS idx
							  FROM descs
					       )
					 WHERE (pi->>'id')::INT8 > (idx->>'id')::INT8
                 )
SELECT database_name,
       schema_name,
       name AS table_name,
       table_id,
       idx->>'name' AS index_name,
       (idx->>'id')::INT8 AS index_id
  FROM crdb_internal.tables JOIN bad_descs ON (table_id = (d->>'id')::INT8);