Skip to content

Commit

Permalink
backupccl: reintroduce previously offline tables with manifest.Descri…
Browse files Browse the repository at this point in the history
…ptorChanges

getReintroducedSpans finds all tables included in the current and previous
backup that may have undergone a non-mvcc operation. The current backup will
then back up these tables' spans from ts = 0, as the previous backup may have
missed certain non-mvcc written.

To find these tables, getReintroducedSpans must find all tables covered in the
previous backup that were in the offline state at previous backup start time.

This function assumed that the previous backup's
manifest.Descriptors field would contain all tables covered in the previous
backup; however, while investigating cockroachdb#88042, we discovered that this assumption
is not correct. During revision history backups, a table with an in-progress
import (e.g. offline at backup time) can get backed up and included in
manifest.DescriptorChanges but not in manifest.Descriptors. This implies that
getReintroducedSpans missed reintroducing spans from this table, implying that
backup chains have missed backing up some data.

This patch fixes getReintroducedSpans to ensure it reintroduces tables included
in manifest.DescriptorChanges whose last revision brought the table offline.

Release note(bug fix): fix apart of Tech Advisory
https://cockroachlabs.atlassian.net/browse/TSE-198

Release justification: bug fix
  • Loading branch information
msbutler committed Sep 23, 2022
1 parent 7823039 commit cbe7526
Showing 1 changed file with 32 additions and 0 deletions.
32 changes: 32 additions & 0 deletions pkg/ccl/backupccl/backup_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -1023,8 +1023,23 @@ func getReintroducedSpans(
) ([]roachpb.Span, error) {
reintroducedTables := make(map[descpb.ID]struct{})

// First, create a map that indicates which tables from the previous backup
// were offline when the last backup was taken. To create this map, we must
// iterate two fields in the _last_ backup's manifest:
//
// 1. manifest.Descriptors contains a list of descriptors _explicitly_
// included in the backup, gathered at backup startTime.
//
// 2. manifest.DescriptorChanges contains a list of descriptor changes tracked
// in the backup. While investigating #88042, it was discovered that
// during revision history backups, a table can get included in
// manifest.DescriptorChanges, causing its spans to get backed up, but _not_
// in manifest.Descriptors. Therefore, to find all descriptors covered in the
// backup that were offline at backup time, we must find all tables in
// manifest.DescriptorChanges whose last change brought the table offline.
offlineInLastBackup := make(map[descpb.ID]struct{})
lastBackup := prevBackups[len(prevBackups)-1]

for _, desc := range lastBackup.Descriptors {
// TODO(pbardea): Also check that lastWriteTime is set once those are
// populated on the table descriptor.
Expand All @@ -1033,6 +1048,23 @@ func getReintroducedSpans(
}
}

latestTableDescChangeInLastBackup := make(map[descpb.ID]*descpb.TableDescriptor)
for _, rev := range lastBackup.DescriptorChanges {
if table, _, _, _, _ := descpb.FromDescriptor(rev.Desc); table != nil {
if trackedRev, ok := latestTableDescChangeInLastBackup[table.GetID()]; !ok {
latestTableDescChangeInLastBackup[table.GetID()] = table
} else if trackedRev.Version < table.Version {
latestTableDescChangeInLastBackup[table.GetID()] = table
}
}
}

for _, table := range latestTableDescChangeInLastBackup {
if table.Offline() {
offlineInLastBackup[table.GetID()] = struct{}{}
}
}

// If the table was offline in the last backup, but becomes PUBLIC, then it
// needs to be re-included since we may have missed non-transactional writes.
tablesToReinclude := make([]catalog.TableDescriptor, 0)
Expand Down

0 comments on commit cbe7526

Please sign in to comment.