Skip to content

Commit

Permalink
Merge #73393 #73652 #73700
Browse files Browse the repository at this point in the history 
73393: backup: refactor resolution work r=dt a=dt

This refactors some of backup's resolution work, done during planning prior to
job creation, into a helper. Specifically, the steps which scan external storage
for prior backups and pick paths, determine coverings, etc, is moved into a helper.

This doesn't change when this runs yet, but is done with an eye towards then moving
the invocation of this to job execution in a follow-up.

Release note: none.

73652: storage: clean up deprecated comment r=AlexTalks a=AlexTalks

This change fixes a deprecated comment which referred to the
`isSeparated` return parameter of the `mvccGetMetadata` function,
which was removed #72536.

Release note: None

73700: ui: update reset index stats button text r=lindseyjin a=lindseyjin

Since the "reset index stats" button currently clears all index stats
for a database, we're updating the button text to be a little more
clear. This can be changed back once resetting by level (index, table,
cluster) is implemented.

<img width="398" alt="Screen Shot 2021-12-10 at 5 21 44 PM" src="https://user-images.githubusercontent.com/29153209/145649181-f7165f67-024e-40c5-893b-05c59e1e85e1.png">
<img width="398" alt="Screen Shot 2021-12-10 at 5 21 37 PM" src="https://user-images.githubusercontent.com/29153209/145649190-99f5ddcf-0648-4c21-9af9-92a2c5d0825e.png">


Release note (ui change): update "reset index stats" button text to be
more clear

Co-authored-by: David Taylor <[email protected]>
Co-authored-by: Alex Sarkesian <[email protected]>
Co-authored-by: Lindsey Jin <[email protected]>
  • Loading branch information
@dt @AlexTalks @lindseyjin
4 people committed Dec 11, 2021
4 parents 8957209 + 3e7249e + c85c469 + e956e32 commit 406e0fe
Show file tree
Hide file tree
Showing 9 changed files with 1,847 additions and 933 deletions.
96 changes: 33 additions & 63 deletions pkg/ccl/backupccl/backup_destination.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"io/ioutil"
"net/url"
"path"
"strings"

"github.com/cockroachdb/cockroach/pkg/ccl/storageccl"
"github.com/cockroachdb/cockroach/pkg/cloud"
Expand All @@ -34,15 +33,16 @@ func fetchPreviousBackups(
user security.SQLUsername,
makeCloudStorage cloud.ExternalStorageFromURIFactory,
prevBackupURIs []string,
encryptionParams backupEncryptionParams,
encryptionParams jobspb.BackupEncryptionOptions,
kmsEnv cloud.KMSEnv,
) ([]BackupManifest, *jobspb.BackupEncryptionOptions, error) {
if len(prevBackupURIs) == 0 {
return nil, nil, nil
}

baseBackup := prevBackupURIs[0]
encryptionOptions, err := getEncryptionFromBase(ctx, user, makeCloudStorage, baseBackup,
encryptionParams)
encryptionParams, kmsEnv)
if err != nil {
return nil, nil, err
}
Expand All @@ -64,22 +64,13 @@ func fetchPreviousBackups(
// explicitly, or due to the auto-append feature), it will resolve the
// encryption options based on the base backup, as well as find all previous
// backup manifests in the backup chain.
//
// TODO(pbardea): Cleanup list for after stability
// - We shouldn't need to pass `to` and (`defaultURI`, `urisByLocalityKV`). We
// can determine the latter from the former.
func resolveDest(
ctx context.Context,
user security.SQLUsername,
nested, appendToLatest bool,
defaultURI string,
urisByLocalityKV map[string]string,
dest jobspb.BackupDetails_Destination,
makeCloudStorage cloud.ExternalStorageFromURIFactory,
endTime hlc.Timestamp,
to []string,
incrementalFrom []string,
subdir string,
incrementalStorage []string,
) (
string, /* collectionURI */
string, /* defaultURI - the full path for the planned backup */
Expand All @@ -95,13 +86,22 @@ func resolveDest(
var chosenSuffix string
var err error

if nested {
collectionURI, chosenSuffix, err = resolveBackupCollection(ctx, user, defaultURI,
appendToLatest, makeCloudStorage, endTime, subdir)
if err != nil {
return "", "", "", nil, nil, err
defaultURI, urisByLocalityKV, err := getURIsByLocalityKV(dest.To, "")
if err != nil {
return "", "", "", nil, nil, err
}

if dest.Subdir != "" {
collectionURI = defaultURI
chosenSuffix = dest.Subdir
if chosenSuffix == latestFileName {
latest, err := readLatestFile(ctx, defaultURI, makeCloudStorage, user)
if err != nil {
return "", "", "", nil, nil, err
}
chosenSuffix = latest
}
defaultURI, urisByLocalityKV, err = getURIsByLocalityKV(to, chosenSuffix)
defaultURI, urisByLocalityKV, err = getURIsByLocalityKV(dest.To, chosenSuffix)
if err != nil {
return "", "", "", nil, nil, err
}
Expand Down Expand Up @@ -129,13 +129,13 @@ func resolveDest(

var priors []string
var backupChainURI string
if len(incrementalStorage) > 0 {
if len(dest.IncrementalStorage) > 0 {
// Implies the incremental backup chain lives in
// incrementalStorage/chosenSuffix, while the full backup lives in
// defaultStore/chosenSuffix. The incremental backup chain in
// incrementalStorage/chosenSuffix will not contain any incremental backups that live
// elsewhere.
backupChainURI, _, err = getURIsByLocalityKV(incrementalStorage,
backupChainURI, _, err = getURIsByLocalityKV(dest.IncrementalStorage,
chosenSuffix)
if err != nil {
return "", "", "", nil, nil, err
Expand Down Expand Up @@ -172,10 +172,10 @@ func resolveDest(
// Within the chosenSuffix dir, differentiate files with partName.
partName := endTime.GoTime().Format(DateBasedIncFolderName)
partName = path.Join(chosenSuffix, partName)
if len(incrementalStorage) > 0 {
defaultURI, urisByLocalityKV, err = getURIsByLocalityKV(incrementalStorage, partName)
if len(dest.IncrementalStorage) > 0 {
defaultURI, urisByLocalityKV, err = getURIsByLocalityKV(dest.IncrementalStorage, partName)
} else {
defaultURI, urisByLocalityKV, err = getURIsByLocalityKV(to, partName)
defaultURI, urisByLocalityKV, err = getURIsByLocalityKV(dest.To, partName)
}
if err != nil {
return "", "", "", nil, nil, errors.Wrap(err, "adjusting backup destination to append new layer to existing backup")
Expand Down Expand Up @@ -235,10 +235,11 @@ func getEncryptionFromBase(
user security.SQLUsername,
makeCloudStorage cloud.ExternalStorageFromURIFactory,
baseBackupURI string,
encryptionParams backupEncryptionParams,
encryptionParams jobspb.BackupEncryptionOptions,
kmsEnv cloud.KMSEnv,
) (*jobspb.BackupEncryptionOptions, error) {
var encryptionOptions *jobspb.BackupEncryptionOptions
if encryptionParams.encryptMode != noEncryption {
if encryptionParams.Mode != jobspb.EncryptionMode_None {
exportStore, err := makeCloudStorage(ctx, baseBackupURI, user)
if err != nil {
return nil, err
Expand All @@ -249,15 +250,15 @@ func getEncryptionFromBase(
return nil, err
}

switch encryptionParams.encryptMode {
case passphrase:
switch encryptionParams.Mode {
case jobspb.EncryptionMode_Passphrase:
encryptionOptions = &jobspb.BackupEncryptionOptions{
Mode: jobspb.EncryptionMode_Passphrase,
Key: storageccl.GenerateKey(encryptionParams.encryptionPassphrase, opts.Salt),
Key: storageccl.GenerateKey([]byte(encryptionParams.RawPassphrae), opts.Salt),
}
case kms:
defaultKMSInfo, err := validateKMSURIsAgainstFullBackup(encryptionParams.kmsURIs,
newEncryptedDataKeyMapFromProtoMap(opts.EncryptedDataKeyByKMSMasterKeyID), encryptionParams.kmsEnv)
case jobspb.EncryptionMode_KMS:
defaultKMSInfo, err := validateKMSURIsAgainstFullBackup(encryptionParams.RawKmsUris,
newEncryptedDataKeyMapFromProtoMap(opts.EncryptedDataKeyByKMSMasterKeyID), kmsEnv)
if err != nil {
return nil, err
}
Expand All @@ -269,37 +270,6 @@ func getEncryptionFromBase(
return encryptionOptions, nil
}

// resolveBackupCollection returns the collectionURI and chosenSuffix that we
// should use for a backup that is pointing to a collection.
func resolveBackupCollection(
ctx context.Context,
user security.SQLUsername,
defaultURI string,
appendToLatest bool,
makeCloudStorage cloud.ExternalStorageFromURIFactory,
endTime hlc.Timestamp,
subdir string,
) (string, string, error) {
var chosenSuffix string
collectionURI := defaultURI

if appendToLatest {
// User called 'BACKUP ... INTO LATEST IN ...', i.e.appendToLatest == True,
latest, err := readLatestFile(ctx, collectionURI, makeCloudStorage, user)
if err != nil {
return "", "", err
}
chosenSuffix = latest
} else if subdir != "" {
// User has specified a subdir via `BACKUP INTO 'subdir' IN...`.
chosenSuffix = strings.TrimPrefix(subdir, "/")
chosenSuffix = "/" + chosenSuffix
} else {
chosenSuffix = endTime.GoTime().Format(DateBasedIntoFolderName)
}
return collectionURI, chosenSuffix, nil
}

func readLatestFile(
ctx context.Context,
collectionURI string,
Expand Down
29 changes: 16 additions & 13 deletions pkg/ccl/backupccl/backup_destination_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/blobs"
"github.com/cockroachdb/cockroach/pkg/cloud"
"github.com/cockroachdb/cockroach/pkg/cloud/nodelocal"
"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
"github.com/cockroachdb/cockroach/pkg/security"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/testutils"
Expand Down Expand Up @@ -137,10 +138,9 @@ func TestBackupRestoreResolveDestination(t *testing.T) {

collectionURI, defaultURI, chosenSuffix, urisByLocalityKV, prevBackupURIs, err := resolveDest(
ctx, security.RootUserName(),
false /* nested */, false, /* appendToLatest */
defaultDest, localitiesDest,
jobspb.BackupDetails_Destination{To: to},
externalStorageFromURI, endTime,
to, incrementalFrom, "", nil,
incrementalFrom,
)
require.NoError(t, err)

Expand Down Expand Up @@ -202,14 +202,11 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
) {
endTime := hlc.Timestamp{WallTime: backupTime.UnixNano()}

dest, localitiesDest, err := getURIsByLocalityKV(to, "")
require.NoError(t, err)
collectionURI, defaultURI, chosenSuffix, urisByLocalityKV, prevBackupURIs, err := resolveDest(
ctx, security.RootUserName(),
false /* nested */, false, /* appendToLatest */
dest, localitiesDest,
jobspb.BackupDetails_Destination{To: to},
externalStorageFromURI, endTime,
to, nil /* incrementalFrom */, "", nil,
nil, /* incrementalFrom */
)
require.NoError(t, err)

Expand Down Expand Up @@ -320,7 +317,13 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
endTime := hlc.Timestamp{WallTime: backupTime.UnixNano()}
incrementalFrom := []string(nil)

defaultCollection, localityCollections, err := getURIsByLocalityKV(collectionTo, "")
if appendToLatest {
subdir = latestFileName
} else if subdir == "" {
subdir = endTime.GoTime().Format(DateBasedIntoFolderName)
}

_, localityCollections, err := getURIsByLocalityKV(collectionTo, "")
require.NoError(t, err)

if len(incrementalTo) > 0 {
Expand All @@ -329,10 +332,10 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
}
collectionURI, defaultURI, chosenSuffix, urisByLocalityKV, prevBackupURIs, err := resolveDest(
ctx, security.RootUserName(),
true /* nested */, appendToLatest,
defaultCollection, localityCollections,
externalStorageFromURI, endTime,
collectionTo, incrementalFrom, subdir, incrementalTo,
jobspb.BackupDetails_Destination{To: collectionTo, Subdir: subdir, IncrementalStorage: incrementalTo},
externalStorageFromURI,
endTime,
incrementalFrom,
)
require.NoError(t, err)

Expand Down
Loading

0 comments on commit 406e0fe

Please sign in to comment.