Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
75451: backupccl,spanconfig,kvserver: ExportRequest noops on ephemeral ranges r=adityamaru a=adityamaru

This change is the first of two changes that gets us to the goal of backup
ignoring certain table row data, and not holding up GC on these ranges.

This change does a few things:

- It sets up the transport of the exclude_data_from_backup bit set on a
table descriptor, to the span configuration applied in KV.

- It teaches ExportRequest on a range marked as excluded to return
an empty ExportResponse. In this way, a backup processor will receive no row
data to backup up for an ephemeral table.

- A follow up change will also teach the SQLTranslator
to not populate the protected timestamp field on the SpanConfig for such
tables. This way, a long running backup will not hold up GC on such high-churn
tables. With no protection on such ranges, it is possible that an
ExportRequest targetting the range has a StartTime
below the range's GCThreshold. To avoid the returned BatchTimestampBeforeGCError
from failing the backup we decorate the the error with information about the
range being excluded from backup and handle the error in the backup processor.

Informs: #73536

Release note (sql change): BACKUP of a table marked with `exclude_data_from_backup`
via `ALTER TABLE ... SET (exclude_data_from_backup = true)` will no longer backup
that table's row data. The backup will continue to backup the table's descriptor
and related metadata, and so on restore we will end up with an empty version of
the backed up table.

76168: ui,server: Table stats collection details added to Database pages r=ericharmeling a=ericharmeling

Closes: #67510

Release justification: low-risk, high benefit changes to
existing functionality

Release note (ui change):
- Added status of automatic statistics collection to the Database
and Database table pages on the DB Console.
- Added timestamp of last statistics collection to the Database
details and Database table pages on the DB Console.

Here's a video of the changes in the UI:

https://user-images.githubusercontent.com/27286675/153521703-9ed19ed0-5fe9-4cb6-a537-1d227aeb0a0b.mov




76444: bazel: upgrade to bazel 5.0.0 r=rail a=rickystewart

Closes #75796.

Release note: None

Co-authored-by: Aditya Maru <[email protected]>
Co-authored-by: Eric Harmeling <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
  • Loading branch information
4 people committed Feb 14, 2022
4 parents 66dfdac + a2aad65 + a41e8c8 + 944d04b commit 47880a9
Show file tree
Hide file tree
Showing 52 changed files with 1,629 additions and 851 deletions.
2 changes: 1 addition & 1 deletion .bazelrc
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
build --define gotags=bazel,gss
build --experimental_proto_descriptor_sets_include_source_info
build --incompatible_strict_action_env --incompatible_enable_cc_toolchain_resolution
build --symlink_prefix=_bazel/ --experimental_no_product_name_out_symlink
build --symlink_prefix=_bazel/
test --config=test --experimental_ui_max_stdouterr_bytes=10485760
build:with_ui --define cockroach_with_ui=y
build:test --define crdb_test=y
Expand Down
2 changes: 1 addition & 1 deletion .bazelversion
Original file line number Diff line number Diff line change
@@ -1 +1 @@
cockroachdb/4.2.1
cockroachdb/5.0.0
1 change: 1 addition & 0 deletions docs/generated/http/full.md
Original file line number Diff line number Diff line change
Expand Up @@ -4603,6 +4603,7 @@ a table.
| zone_config_level | [ZoneConfigurationLevel](#cockroach.server.serverpb.TableDetailsResponse-cockroach.server.serverpb.ZoneConfigurationLevel) | | The level at which this object's zone configuration is set. | [reserved](#support-status) |
| descriptor_id | [int64](#cockroach.server.serverpb.TableDetailsResponse-int64) | | descriptor_id is an identifier used to uniquely identify this table. It can be used to find events pertaining to this table by filtering on the 'target_id' field of events. | [reserved](#support-status) |
| configure_zone_statement | [string](#cockroach.server.serverpb.TableDetailsResponse-string) | | configure_zone_statement is the output of "SHOW ZONE CONFIGURATION FOR TABLE" for this table. It is a SQL statement that would re-configure the table's current zone if executed. | [reserved](#support-status) |
| stats_last_created_at | [google.protobuf.Timestamp](#cockroach.server.serverpb.TableDetailsResponse-google.protobuf.Timestamp) | | stats_last_created_at is the time at which statistics were last created. | [reserved](#support-status) |



Expand Down
6 changes: 6 additions & 0 deletions docs/generated/swagger/spec.json
Original file line number Diff line number Diff line change
Expand Up @@ -1279,6 +1279,12 @@
"format": "int64",
"x-go-name": "RangeCount"
},
"stats_last_created_at": {
"description": "stats_last_created_at is the time at which statistics were last created.",
"type": "string",
"format": "date-time",
"x-go-name": "StatsLastCreatedAt"
},
"zone_config": {
"$ref": "#/definitions/ZoneConfig"
},
Expand Down
8 changes: 6 additions & 2 deletions pkg/ccl/backupccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ go_library(
"//pkg/sql/catalog/descidgen",
"//pkg/sql/catalog/descpb",
"//pkg/sql/catalog/descs",
"//pkg/sql/catalog/desctestutils",
"//pkg/sql/catalog/multiregion",
"//pkg/sql/catalog/resolver",
"//pkg/sql/catalog/schemadesc",
Expand Down Expand Up @@ -103,6 +104,9 @@ go_library(
"//pkg/sql/types",
"//pkg/storage",
"//pkg/testutils",
"//pkg/testutils/serverutils",
"//pkg/testutils/sqlutils",
"//pkg/testutils/testcluster",
"//pkg/util",
"//pkg/util/admission",
"//pkg/util/contextutil",
Expand All @@ -123,6 +127,8 @@ go_library(
"//pkg/util/timeutil",
"//pkg/util/tracing",
"//pkg/util/uuid",
"//pkg/workload/bank",
"//pkg/workload/workloadsql",
"@com_github_cockroachdb_errors//:errors",
"@com_github_cockroachdb_logtags//:logtags",
"@com_github_gogo_protobuf//jsonpb",
Expand All @@ -148,7 +154,6 @@ go_test(
"bench_test.go",
"create_scheduled_backup_test.go",
"full_cluster_backup_restore_test.go",
"helpers_test.go",
"insert_missing_public_schema_namespace_entry_restore_test.go",
"key_rewriter_test.go",
"main_test.go",
Expand Down Expand Up @@ -247,7 +252,6 @@ go_test(
"//pkg/util/timeutil",
"//pkg/util/uuid",
"//pkg/workload/bank",
"//pkg/workload/workloadsql",
"@com_github_aws_aws_sdk_go//aws/credentials",
"@com_github_cockroachdb_cockroach_go_v2//crdb",
"@com_github_cockroachdb_datadriven//:datadriven",
Expand Down
6 changes: 3 additions & 3 deletions pkg/ccl/backupccl/backup_cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func TestCloudBackupRestoreS3(t *testing.T) {
const numAccounts = 1000

ctx := context.Background()
tc, _, _, cleanupFn := BackupRestoreTestSetup(t, 1, numAccounts, InitManualReplication)
tc, _, _, cleanupFn := backupRestoreTestSetup(t, 1, numAccounts, InitManualReplication)
defer cleanupFn()
prefix := fmt.Sprintf("TestBackupRestoreS3-%d", timeutil.Now().UnixNano())
uri := url.URL{Scheme: "s3", Host: bucket, Path: prefix}
Expand All @@ -89,7 +89,7 @@ func TestCloudBackupRestoreGoogleCloudStorage(t *testing.T) {
const numAccounts = 1000

ctx := context.Background()
tc, _, _, cleanupFn := BackupRestoreTestSetup(t, 1, numAccounts, InitManualReplication)
tc, _, _, cleanupFn := backupRestoreTestSetup(t, 1, numAccounts, InitManualReplication)
defer cleanupFn()
prefix := fmt.Sprintf("TestBackupRestoreGoogleCloudStorage-%d", timeutil.Now().UnixNano())
uri := url.URL{Scheme: "gs", Host: bucket, Path: prefix}
Expand Down Expand Up @@ -120,7 +120,7 @@ func TestCloudBackupRestoreAzure(t *testing.T) {
const numAccounts = 1000

ctx := context.Background()
tc, _, _, cleanupFn := BackupRestoreTestSetup(t, 1, numAccounts, InitManualReplication)
tc, _, _, cleanupFn := backupRestoreTestSetup(t, 1, numAccounts, InitManualReplication)
defer cleanupFn()
prefix := fmt.Sprintf("TestBackupRestoreAzure-%d", timeutil.Now().UnixNano())
uri := url.URL{Scheme: "azure", Host: bucket, Path: prefix}
Expand Down
12 changes: 11 additions & 1 deletion pkg/ccl/backupccl/backup_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,17 @@ func runBackupProcessor(
if errors.HasType(exportRequestErr, (*contextutil.TimeoutError)(nil)) {
return errors.Wrap(exportRequestErr, "export request timeout")
}
// BatchTimestampBeforeGCError is returned if the ExportRequest
// attempts to read below the range's GC threshold.
if batchTimestampBeforeGCError, ok := pErr.GetDetail().(*roachpb.BatchTimestampBeforeGCError); ok {
// If the range we are exporting is marked to be excluded from
// backup, it is safe to ignore the error. It is likely that the
// table has been configured with a low GC TTL, and so the data
// the backup is targeting has already been gc'ed.
if batchTimestampBeforeGCError.DataExcludedFromBackup {
continue
}
}
return errors.Wrapf(exportRequestErr, "exporting %s", span.span)
}

Expand Down Expand Up @@ -633,7 +644,6 @@ func makeSSTSink(
// prior to the error.
incrementSize := int64(32 << 20)
maxSize := smallFileBuffer.Get(s.conf.settings)
log.Infof(ctx, "this is the max size we are using %d", (maxSize / (1024 * 1024)))
for {
if s.queueCap >= maxSize {
break
Expand Down
6 changes: 3 additions & 3 deletions pkg/ccl/backupccl/backup_rand_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ database_name = 'rand' AND schema_name = 'public'`)
// Now that we've created our random tables, backup and restore the whole DB
// and compare all table descriptors for equality.

dbBackup := LocalFoo + "wholedb"
tablesBackup := LocalFoo + "alltables"
dbBackup := localFoo + "wholedb"
tablesBackup := localFoo + "alltables"
dbBackups := []string{dbBackup, tablesBackup}

if err := verifyBackupRestoreStatementResult(
Expand Down Expand Up @@ -136,7 +136,7 @@ database_name = 'rand' AND schema_name = 'public'`)

for i, combo := range tableNameCombos {
sqlDB.Exec(t, "DROP DATABASE IF EXISTS restoredb; CREATE DATABASE restoredb")
backupTarget := fmt.Sprintf("%s%d", LocalFoo, i)
backupTarget := fmt.Sprintf("%s%d", localFoo, i)
if len(combo) == 0 {
continue
}
Expand Down
Loading

0 comments on commit 47880a9

Please sign in to comment.