backupccl: only check for introduced spans for online tables in restore

This small patch refines the introduced span corruption checker to
ensure that only _online_ tables in backup i, are checked, as oppose to all
tables included in manifest.Descriptors. This is necessary for the 22.2 patch,
since in 22.2, all backups back up offline descriptors.

pkg/ccl/backupccl/targets.go

@@ -342,7 +342,7 @@ func fullClusterTargetsBackup(
 // they are also restore targets. This specifically entails checking the
 // following invariant on each table we seek to restore: if the nth backup
 // contains an online table in its backupManifest.Descriptors field or
-// backupManifest. DescriptorsChanges field but the n-1th does not contain it in
+// backupManifest.DescriptorsChanges field but the n-1th does not contain it online in
 // its Descriptors field, then that table must be covered by the nth backup's
 // manifest.IntroducedSpans field. Note: this check assumes that
 // mainBackupManifests are sorted by Endtime and this check only applies to
@@ -372,11 +372,11 @@ func checkMissingIntroducedSpans(
 			continue
 		}
 
-		// Gather the tables included in the previous backup.
-		prevTables := make(map[descpb.ID]struct{})
+		// Gather the _online_ tables included in the previous backup.
+		prevOnlineTables := make(map[descpb.ID]struct{})
 		for _, desc := range mainBackupManifests[i-1].Descriptors {
-			if table, _, _, _, _ := descpb.FromDescriptor(&desc); table != nil {
-				prevTables[table.GetID()] = struct{}{}
+			if table, _, _, _, _ := descpb.FromDescriptor(&desc); table != nil && table.Public() {
+				prevOnlineTables[table.GetID()] = struct{}{}
 			}
 		}
 
@@ -391,6 +391,8 @@ func checkMissingIntroducedSpans(
 			tablesIntroduced[descpb.ID(tableID)] = struct{}{}
 		}
 
+		// requiredIntroduction affirms that the given table in backup i was either
+		// introduced in backup i, or included online in backup i-1.
 		requiredIntroduction := func(table *descpb.TableDescriptor) error {
 			if _, required := requiredTables[table.GetID()]; !required {
 				// The table is not required for the restore, so there's no need to check coverage.
@@ -407,7 +409,7 @@ func checkMissingIntroducedSpans(
 				return nil
 			}
 
-			if _, inPrevBackup := prevTables[table.GetID()]; inPrevBackup {
+			if _, inPrevBackup := prevOnlineTables[table.GetID()]; inPrevBackup {
 				// The table was included in the previous backup, which implies the
 				// table was online at prevBackup.Endtime, and therefore does not need
 				// to be introduced.
@@ -428,20 +430,20 @@ that was running an IMPORT at the time of the previous incremental in this chain
 			})
 		}
 
-		// If the table in the current backup was not in prevBackup.Descriptors,
-		// then it needs to be introduced.
 		for _, desc := range mainBackupManifests[i].Descriptors {
-			if table, _, _, _, _ := descpb.FromDescriptor(&desc); table != nil {
+			// Check that all online tables at backup time were either introduced or
+			// in the previous backup.
+			if table, _, _, _, _ := descpb.FromDescriptor(&desc); table != nil && table.Public() {
 				if err := requiredIntroduction(table); err != nil {
 					return err
 				}
 			}
 		}
 
 		// As described in the docstring for getReintroducedSpans(), there are cases
-		// where a descriptor may appear in manifest. DescriptorChanges but not
-		// manifest.Descriptors. If a descriptor is online at any moment during the
-		// backup interval, it needs to be reintroduced.
+		// where a descriptor may appear in manifest.DescriptorChanges but not
+		// manifest.Descriptors. If a descriptor switched from offline to online at
+		// any moment during the backup interval, it needs to be reintroduced.
 		for _, desc := range mainBackupManifests[i].DescriptorChanges {
 			if table, _, _, _, _ := descpb.FromDescriptor(desc.Desc); table != nil && table.Public() {
 				if err := requiredIntroduction(table); err != nil {

pkg/ccl/backupccl/testdata/backup-restore/in-progress-import-rollback

@@ -104,11 +104,26 @@ SELECT count(*) FROM d.foofoo;
 ----
 1
 
+# Start and pause another import to ensure the restore checker doesn't spuriously fail on
+# descriptors with an in-progress import.
+exec-sql
+SET CLUSTER SETTING jobs.debug.pausepoints = 'import.after_ingest';
+----
+
+exec-sql
+CREATE TABLE foo_offline (i INT PRIMARY KEY, s STRING);
+----
+
+import expect-pausepoint tag=never_unpause
+IMPORT INTO foo_offline (i,s) CSV DATA ('nodelocal://0/export1/export*-n*.0.csv')
+----
+job paused at pausepoint
+
 
 # Even though the full table will get backed up from ts=0 during the next round of incremental
 # backups, only active indexes (foo_idx and foo_new_idx) should appear in the restored cluster.
 exec-sql
-DROP INDEX foo_to_drop_idx;
+DROP INDEX foo@foo_to_drop_idx;
 ----
 NOTICE: the data for dropped indexes is reclaimed asynchronously
 HINT: The reclamation delay can be customized in the zone configuration for the table.
@@ -284,7 +299,7 @@ job tag=bb wait-for-state=cancelled
 # Even though the full table will get backed up from ts=0 during the next round of incremental
 # backups, only active indexes (foo2_idx and foo2_new_idx) should appear in the restored cluster.
 exec-sql
-DROP INDEX foo2_to_drop_idx;
+DROP INDEX foo2@foo2_to_drop_idx;
 ----
 NOTICE: the data for dropped indexes is reclaimed asynchronously
 HINT: The reclamation delay can be customized in the zone configuration for the table.