Skip to content

Commit

Permalink
spanconfigsqltranslator,jobsprotectedts: add ignore_if_excluded_from_…
Browse files Browse the repository at this point in the history 
…backup to SpanConfig

This change is a follow up to cockroachdb#75451 which taught ExportRequests
to noop on ranges marked as exclude_data_from_backup. This change

This diff does two things:

- It adds an `ignore_if_excluded_from_backup` bit to ptpb.Target that is set
on PTS records written by backup schedules and jobs.

- It adds an `ignore_if_excluded_from_backup` bit to the ProtectionPolicy that
is shipped to KV as part of the SpanConfig.

In a follow up PR, this bit on the SpanConfig will be used in conjunction with
`exclude_data_from_backup` to decide whether or not to ignore the ProtectionPolicy
when making GC decisions on a span. All other consumers of PTS records will
default to setting this bit to false, and so their ProtectionPolicies will always
influence GC even if `exclude_data_from_backup` is set to true.

Informs: cockroachdb#73536

Release note: None
  • Loading branch information
@adityamaru @maryliag
adityamaru authored and maryliag committed Feb 28, 2022
1 parent f1f8981 commit ab739f7
Show file tree
Hide file tree
Showing 15 changed files with 140 additions and 41 deletions.
6 changes: 6 additions & 0 deletions pkg/ccl/backupccl/backup_planning.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1341,6 +1341,12 @@ func protectTimestampForBackup(
// Resolve the target that the PTS record will protect as part of this
// backup.
target := getProtectedTimestampTargetForBackup(backupManifest)

// Records written by the backup job should be ignored when making GC
// decisions on any table that has been marked as
// `exclude_data_from_backup`. This ensures that the backup job does not
// holdup GC on that table span for the duration of execution.
target.IgnoreIfExcludedFromBackup = true
rec := jobsprotectedts.MakeRecord(*backupDetails.ProtectedTimestampRecord, int64(jobID),
tsToProtect, backupManifest.Spans, jobsprotectedts.Jobs, target)
err := execCfg.ProtectedTimestampProvider.Protect(ctx, txn, rec)
Expand Down
8 changes: 8 additions & 0 deletions pkg/ccl/backupccl/schedule_pts_chaining.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,14 @@ func manageFullBackupPTSChaining(
return errors.Wrap(err, "getting spans to protect")
}

// Records written by the backup schedule should be ignored when making GC
// decisions on any table that has been marked as `exclude_data_from_backup`.
// This ensures that the schedule does not holdup GC on that table span for
// the duration of execution.
if targetToProtect != nil {
targetToProtect.IgnoreIfExcludedFromBackup = true
}

// Protect the target after the EndTime of the current backup. We do not need
// to verify this new record as we have a record written by the backup during
// planning, already protecting this target after EndTime.
Expand Down
4 changes: 3 additions & 1 deletion pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/datadriven_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,11 @@ func TestDataDriven(t *testing.T) {
var protectTS int
d.ScanArgs(t, "record-id", &recordID)
d.ScanArgs(t, "ts", &protectTS)
target := spanconfigtestutils.ParseProtectionTarget(t, d.Input)

jobID := tenant.JobsRegistry().MakeJobID()
target := spanconfigtestutils.ParseProtectionTarget(t, d.Input)
target.IgnoreIfExcludedFromBackup = d.HasArg("ignore-if-excluded-from-backup")

require.NoError(t, tenant.ExecCfg().DB.Txn(ctx,
func(ctx context.Context, txn *kv.Txn) (err error) {
require.Len(t, recordID, 1,
Expand Down
25 changes: 23 additions & 2 deletions pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/exclude_data_from_backup
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,33 @@ translate database=db table=t2
----
/Table/10{7-8} range default

# Write a protection record as a "backup" to test the translation of the
# `ignore_if_excluded_from_backup` bit on the ProtectionPolicy.
protect record-id=1 ts=1 ignore-if-excluded-from-backup
descs 104
----

# Write another protection record as a non-backup user.
protect record-id=2 ts=2
descs 104
----

# Translate to ensure that the ProtectionPolicy is set with
# `ignore_if_excluded_from_backup` for the record written by the backup only.
translate database=db
----
/Table/10{6-7} protection_policies=[{ts: 1,ignore_if_excluded_from_backup: true} {ts: 2}] exclude_data_from_backup=true
/Table/10{7-8} protection_policies=[{ts: 1,ignore_if_excluded_from_backup: true} {ts: 2}]

# Alter table t1 to unmark its data ephemeral.
exec-sql
ALTER TABLE db.t1 SET (exclude_data_from_backup = false);
----

release record-id=1
----

translate database=db
----
/Table/10{6-7} range default
/Table/10{7-8} range default
/Table/10{6-7} protection_policies=[{ts: 2}]
/Table/10{7-8} protection_policies=[{ts: 2}]
20 changes: 10 additions & 10 deletions pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/protectedts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ descs 106

translate database=db
----
/Table/10{6-7} num_replicas=7 num_voters=5 pts=[1]
/Table/10{6-7} num_replicas=7 num_voters=5 protection_policies=[{ts: 1}]
/Table/10{7-8} num_replicas=7

# Write a protected timestamp on db, so we should see it on both t1 and t2.
Expand All @@ -36,8 +36,8 @@ descs 104

translate database=db
----
/Table/10{6-7} num_replicas=7 num_voters=5 pts=[1 2]
/Table/10{7-8} num_replicas=7 pts=[2]
/Table/10{6-7} num_replicas=7 num_voters=5 protection_policies=[{ts: 1} {ts: 2}]
/Table/10{7-8} num_replicas=7 protection_policies=[{ts: 2}]

# Write a protected timestamp on the cluster.
protect record-id=3 ts=3
Expand All @@ -63,8 +63,8 @@ translate system-span-configurations

translate database=db
----
/Table/10{6-7} num_replicas=7 num_voters=5 pts=[1 2]
/Table/10{7-8} num_replicas=7 pts=[2]
/Table/10{6-7} num_replicas=7 num_voters=5 protection_policies=[{ts: 1} {ts: 2}]
/Table/10{7-8} num_replicas=7 protection_policies=[{ts: 2}]

# Release the protected timestamp on table t1
release record-id=1
Expand All @@ -79,8 +79,8 @@ translate system-span-configurations

translate database=db
----
/Table/10{6-7} num_replicas=7 num_voters=5 pts=[2]
/Table/10{7-8} num_replicas=7 pts=[2]
/Table/10{6-7} num_replicas=7 num_voters=5 protection_policies=[{ts: 2}]
/Table/10{7-8} num_replicas=7 protection_policies=[{ts: 2}]

# Release the protected timestamp on database db
release record-id=2
Expand Down Expand Up @@ -123,7 +123,7 @@ translate system-span-configurations

translate database=db
----
/Table/106{-/2} num_replicas=7 num_voters=5 pts=[6]
/Table/106/{2-3} ttl_seconds=1 num_replicas=7 num_voters=5 pts=[6]
/Table/10{6/3-7} num_replicas=7 num_voters=5 pts=[6]
/Table/106{-/2} num_replicas=7 num_voters=5 protection_policies=[{ts: 6}]
/Table/106/{2-3} ttl_seconds=1 num_replicas=7 num_voters=5 protection_policies=[{ts: 6}]
/Table/10{6/3-7} num_replicas=7 num_voters=5 protection_policies=[{ts: 6}]
/Table/10{7-8} num_replicas=7
23 changes: 21 additions & 2 deletions pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/tenant/exclude_data_from_backup
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,31 @@ translate database=db table=t2
----
/Tenant/10/Table/10{7-8} range default


# Write a protection record as a "backup" to test the translation of the
# `ignore_if_excluded_from_backup` bit on the ProtectionPolicy.
protect record-id=1 ts=1 ignore-if-excluded-from-backup
descs 104
----

# Write another protection record as a non-backup user.
protect record-id=2 ts=2
descs 104
----

# Translate to ensure that the ProtectionPolicy is set with
# `ignore_if_excluded_from_backup` for the record written by the backup only.
translate database=db
----
/Tenant/10/Table/10{6-7} protection_policies=[{ts: 1,ignore_if_excluded_from_backup: true} {ts: 2}] exclude_data_from_backup=true
/Tenant/10/Table/10{7-8} protection_policies=[{ts: 1,ignore_if_excluded_from_backup: true} {ts: 2}]

# Alter table t1 to unmark its data ephemeral.
exec-sql
ALTER TABLE db.t1 SET (exclude_data_from_backup = false);
----

translate database=db
----
/Tenant/10/Table/10{6-7} range default
/Tenant/10/Table/10{7-8} range default
/Tenant/10/Table/10{6-7} protection_policies=[{ts: 1,ignore_if_excluded_from_backup: true} {ts: 2}]
/Tenant/10/Table/10{7-8} protection_policies=[{ts: 1,ignore_if_excluded_from_backup: true} {ts: 2}]
20 changes: 10 additions & 10 deletions pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/tenant/protectedts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ descs 106

translate database=db
----
/Tenant/10/Table/10{6-7} num_replicas=7 num_voters=5 pts=[1]
/Tenant/10/Table/10{6-7} num_replicas=7 num_voters=5 protection_policies=[{ts: 1}]
/Tenant/10/Table/10{7-8} num_replicas=7

# Write a protected timestamp on db, so we should see it on both t1 and t2.
Expand All @@ -36,8 +36,8 @@ descs 104

translate database=db
----
/Tenant/10/Table/10{6-7} num_replicas=7 num_voters=5 pts=[1 2]
/Tenant/10/Table/10{7-8} num_replicas=7 pts=[2]
/Tenant/10/Table/10{6-7} num_replicas=7 num_voters=5 protection_policies=[{ts: 1} {ts: 2}]
/Tenant/10/Table/10{7-8} num_replicas=7 protection_policies=[{ts: 2}]


# Write a protected timestamp on the tenant cluster.
Expand All @@ -56,8 +56,8 @@ translate system-span-configurations

translate database=db
----
/Tenant/10/Table/10{6-7} num_replicas=7 num_voters=5 pts=[1 2]
/Tenant/10/Table/10{7-8} num_replicas=7 pts=[2]
/Tenant/10/Table/10{6-7} num_replicas=7 num_voters=5 protection_policies=[{ts: 1} {ts: 2}]
/Tenant/10/Table/10{7-8} num_replicas=7 protection_policies=[{ts: 2}]

# Release the protected timestamp on table t1
release record-id=1
Expand All @@ -69,8 +69,8 @@ translate system-span-configurations

translate database=db
----
/Tenant/10/Table/10{6-7} num_replicas=7 num_voters=5 pts=[2]
/Tenant/10/Table/10{7-8} num_replicas=7 pts=[2]
/Tenant/10/Table/10{6-7} num_replicas=7 num_voters=5 protection_policies=[{ts: 2}]
/Tenant/10/Table/10{7-8} num_replicas=7 protection_policies=[{ts: 2}]

# Release the protected timestamp on database db
release record-id=2
Expand Down Expand Up @@ -105,7 +105,7 @@ translate system-span-configurations

translate database=db
----
/Tenant/10/Table/106{-/2} num_replicas=7 num_voters=5 pts=[5]
/Tenant/10/Table/106/{2-3} ttl_seconds=1 num_replicas=7 num_voters=5 pts=[5]
/Tenant/10/Table/10{6/3-7} num_replicas=7 num_voters=5 pts=[5]
/Tenant/10/Table/106{-/2} num_replicas=7 num_voters=5 protection_policies=[{ts: 5}]
/Tenant/10/Table/106/{2-3} ttl_seconds=1 num_replicas=7 num_voters=5 protection_policies=[{ts: 5}]
/Tenant/10/Table/10{6/3-7} num_replicas=7 num_voters=5 protection_policies=[{ts: 5}]
/Tenant/10/Table/10{7-8} num_replicas=7
4 changes: 2 additions & 2 deletions pkg/kv/kvserver/protectedts/protectedts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,10 @@ type Storage interface {
// passed txn remains safe for future use.
Release(context.Context, *kv.Txn, uuid.UUID) error

// GetMetadata retreives the metadata with the provided Txn.
// GetMetadata retrieves the metadata with the provided Txn.
GetMetadata(context.Context, *kv.Txn) (ptpb.Metadata, error)

// GetState retreives the entire state of protectedts.Storage with the
// GetState retrieves the entire state of protectedts.Storage with the
// provided Txn.
GetState(context.Context, *kv.Txn) (ptpb.State, error)

Expand Down
6 changes: 3 additions & 3 deletions pkg/kv/kvserver/protectedts/ptpb/protectedts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,17 @@ import (
// MakeClusterTarget returns a target, which when used in a Record, will
// protect the entire keyspace of the cluster.
func MakeClusterTarget() *Target {
return &Target{&Target_Cluster{Cluster: &Target_ClusterTarget{}}}
return &Target{Union: &Target_Cluster{Cluster: &Target_ClusterTarget{}}}
}

// MakeTenantsTarget returns a target, which when used in a Record, will
// protect the keyspace of all tenants in ids.
func MakeTenantsTarget(ids []roachpb.TenantID) *Target {
return &Target{&Target_Tenants{Tenants: &Target_TenantsTarget{IDs: ids}}}
return &Target{Union: &Target_Tenants{Tenants: &Target_TenantsTarget{IDs: ids}}}
}

// MakeSchemaObjectsTarget returns a target, which when used in a Record,
// will protect the keyspace of all schema objects (database/table).
func MakeSchemaObjectsTarget(ids descpb.IDs) *Target {
return &Target{&Target_SchemaObjects{SchemaObjects: &Target_SchemaObjectsTarget{IDs: ids}}}
return &Target{Union: &Target_SchemaObjects{SchemaObjects: &Target_SchemaObjectsTarget{IDs: ids}}}
}
11 changes: 11 additions & 0 deletions pkg/kv/kvserver/protectedts/ptpb/protectedts.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,4 +173,15 @@ message Target {
TenantsTarget tenants = 2;
ClusterTarget cluster = 3;
}

// IgnoreIfExcludedFromBackup is set to true if the Record can be ignored when
// making GC decisions on a table that has been marked to be excluded from
// backups i.e. the table has `exclude_data_from_backup = true`.
//
// This field is currently only set to true when a protected timestamp record
// has been written by a backup job. This is to ensure that Records written by
// non-backup users (CDC, streaming) on spans marked as
// `exclude_data_from_backup` are still respected when making GC decisions on
// the span.
bool ignore_if_excluded_from_backup = 4;
}
3 changes: 2 additions & 1 deletion pkg/kv/kvserver/protectedts/ptstorage/storage.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,8 @@ func (p *storage) Protect(ctx context.Context, txn *kv.Txn, r *ptpb.Record) erro
// already verified that the record has a valid `target`.
r.DeprecatedSpans = nil
s := makeSettings(p.settings)
encodedTarget, err := protoutil.Marshal(&ptpb.Target{Union: r.Target.GetUnion()})
encodedTarget, err := protoutil.Marshal(&ptpb.Target{Union: r.Target.GetUnion(),
IgnoreIfExcludedFromBackup: r.Target.IgnoreIfExcludedFromBackup})
if err != nil { // how can this possibly fail?
return errors.Wrap(err, "failed to marshal spans")
}
Expand Down
12 changes: 12 additions & 0 deletions pkg/roachpb/span_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,18 @@ func (c ConstraintsConjunction) String() string {
return sb.String()
}

// String implements the stringer interface.
func (p ProtectionPolicy) String() string {
var sb strings.Builder
sb.WriteString(fmt.Sprintf("{ts: %d", int(p.ProtectedTimestamp.WallTime)))
if p.IgnoreIfExcludedFromBackup {
sb.WriteString(fmt.Sprintf(",ignore_if_excluded_from_backup: %t",
p.IgnoreIfExcludedFromBackup))
}
sb.WriteString("}")
return sb.String()
}

// TestingDefaultSpanConfig exports the default span config for testing purposes.
func TestingDefaultSpanConfig() SpanConfig {
return SpanConfig{
Expand Down
13 changes: 13 additions & 0 deletions pkg/roachpb/span_config.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,24 @@ message GCPolicy {
// applies over a given span.
message ProtectionPolicy {
option (gogoproto.equal) = true;
option (gogoproto.goproto_stringer) = false;
option (gogoproto.populate) = true;

// ProtectedTimestamp is a timestamp above which GC should not run, regardless
// of the GC TTL.
util.hlc.Timestamp protected_timestamp = 1 [(gogoproto.nullable) = false];

// IgnoreIfExcludedFromBackup is set to true if the ProtectionPolicy can be
// ignored when making GC decisions on a span that has been marked to be
// excluded from backups i.e. the applied SpanConfig has
// `exclude_data_from_backup = true`.
//
// This field is currently only set to true when a protected timestamp record
// has been written by a backup schedule or job. This is to ensure that
// ProtectionPolicies written by non-backup users (CDC, streaming) on spans
// marked as `exclude_data_from_backup` are still respected when making GC
// decisions on the span.
bool ignore_if_excluded_from_backup = 2;
}

// Constraint constrains the stores that a replica can be stored on. It
Expand Down
18 changes: 12 additions & 6 deletions pkg/spanconfig/spanconfigsqltranslator/protectedts_state_reader.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,19 +73,25 @@ func (p *protectedTimestampStateReader) getProtectionPoliciesForSchemaObject(
func (p *protectedTimestampStateReader) loadProtectedTimestampRecords(ptsState ptpb.State) {
tenantProtections := make(map[roachpb.TenantID][]roachpb.ProtectionPolicy)
for _, record := range ptsState.Records {
// TODO(adityamaru): We should never see this post 22.1 since all records
// will be written with a target.
if record.Target == nil {
continue
}
protectionPolicy := roachpb.ProtectionPolicy{
ProtectedTimestamp: record.Timestamp,
IgnoreIfExcludedFromBackup: record.Target.IgnoreIfExcludedFromBackup,
}
switch t := record.Target.GetUnion().(type) {
case *ptpb.Target_Cluster:
p.clusterProtections = append(p.clusterProtections,
roachpb.ProtectionPolicy{ProtectedTimestamp: record.Timestamp})
p.clusterProtections = append(p.clusterProtections, protectionPolicy)
case *ptpb.Target_Tenants:
for _, tenID := range t.Tenants.IDs {
tenantProtections[tenID] = append(tenantProtections[tenID],
roachpb.ProtectionPolicy{ProtectedTimestamp: record.Timestamp})
tenantProtections[tenID] = append(tenantProtections[tenID], protectionPolicy)
}
case *ptpb.Target_SchemaObjects:
for _, descID := range t.SchemaObjects.IDs {
p.schemaObjectProtections[descID] = append(p.schemaObjectProtections[descID],
roachpb.ProtectionPolicy{ProtectedTimestamp: record.Timestamp})
p.schemaObjectProtections[descID] = append(p.schemaObjectProtections[descID], protectionPolicy)
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/spanconfig/spanconfigtestutils/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,11 +369,11 @@ func PrintSpanConfigDiffedAgainstDefaults(conf roachpb.SpanConfig) string {
rhs := conf.GCPolicy.ProtectionPolicies[j].ProtectedTimestamp
return lhs.Less(rhs)
})
timestamps := make([]string, 0, len(conf.GCPolicy.ProtectionPolicies))
for _, pts := range conf.GCPolicy.ProtectionPolicies {
timestamps = append(timestamps, strconv.Itoa(int(pts.ProtectedTimestamp.WallTime)))
protectionPolicies := make([]string, 0, len(conf.GCPolicy.ProtectionPolicies))
for _, pp := range conf.GCPolicy.ProtectionPolicies {
protectionPolicies = append(protectionPolicies, pp.String())
}
diffs = append(diffs, fmt.Sprintf("pts=[%s]", strings.Join(timestamps, " ")))
diffs = append(diffs, fmt.Sprintf("protection_policies=[%s]", strings.Join(protectionPolicies, " ")))
}
if conf.ExcludeDataFromBackup != defaultConf.ExcludeDataFromBackup {
diffs = append(diffs, fmt.Sprintf("exclude_data_from_backup=%v", conf.ExcludeDataFromBackup))
Expand Down

0 comments on commit ab739f7

Please sign in to comment.