Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
108863: streamingest: periodically output replication-frontier.txt file r=msbutler a=adityamaru

Please see individual commits for details.

Informs: #108374

111275: gc: separate out lock table scan in the gc queue r=nvanbenschoten a=arulajmani

Previously, we'd handle resolving intents while scanning through the
MVCC keyspace if we came across one. We would resolve it if it was
older than some configurable threshold. In all this, we never needed
to scan the lock table keyspace directly.

We're introducing replicated shared and exclusive locks. The GC queue
is expected to resolve extant replicated locks, and as such, needs to
concern itself with an explicit lock table scan. This patch removes
intent handling from the scan of the MVCC keyspace. For now, no behavior
changes -- we only look for and resolve intents. In a future patch we'll
extend this behavior to include shareed and exclusive replicated locks.

While here, we also modify an exsiting test
(`TestIntentAgeThresholdSetting`) to include an intent on a range local
key. I've ensured that the test fails if we were to only scan the lock
table corresponding to the global keyspace of a range.

Informs #111215

Release note: None

111356: ccl: syntax change restore option strip_localities to remove_regions r=msbutler a=annrpom

This changes the syntax from newly added RESTORE option, strip_localities, to remove_regions. The former was inadequate because it was too similar to an existing option, along with not being an accurately descriptive enough name.

Epic: none
Informs: #111348
Part of: https://cockroachlabs.atlassian.net/browse/CRDB-29129

Release note (sql change): The syntax for RESTORE option, strip_localities, is now remove_regions.

111376: dev: remove `merge-test-xmls` command r=rail a=rickystewart

This was used in old-style `stress`. No need for it now that `dev test --stress` uses `--runs_per_test`.

Epic: none
Release note: None

111413: roachtest: remove backup creation during node upgrades r=aliher1911 a=aliher1911

In upgrade tests existing nodes try to start backup schedule on every binary restart. This is not adding any value as this feature tests that cluster will run backups created at initial cluster start.
At the same time those checks pollute logs constantly. This commit changes options for node restarts in upgrade tests to exclude backups.

Epic: none

Release note: None

111417: logic: Fix a few incorrect syntax in `scrub` logic test r=Xiang-Gu a=Xiang-Gu

This commit fixes a few incorrect syntax in `scrub` logic test.

Epic: None
Release note: None

Co-authored-by: adityamaru <[email protected]>
Co-authored-by: Arul Ajmani <[email protected]>
Co-authored-by: Annie Pompa <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Oleg Afanasyev <[email protected]>
Co-authored-by: Xiang Gu <[email protected]>
  • Loading branch information
7 people committed Sep 28, 2023
7 parents 6e49f7f + 5ac0db3 + 6b9599d + 101b569 + 5f975b4 + 7b1953b + 88f1fb9 commit 2ac6153
Show file tree
Hide file tree
Showing 32 changed files with 917 additions and 182 deletions.
2 changes: 1 addition & 1 deletion docs/generated/sql/bnf/restore_options.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ restore_options ::=
| 'UNSAFE_RESTORE_INCOMPATIBLE_VERSION'
| 'EXECUTION' 'LOCALITY' '=' string_or_placeholder
| 'EXPERIMENTAL' 'DEFERRED' 'COPY'
| 'STRIP_LOCALITIES'
| 'REMOVE_REGIONS'
6 changes: 3 additions & 3 deletions docs/generated/sql/bnf/stmt_block.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -1332,6 +1332,7 @@ unreserved_keyword ::=
| 'RELATIVE'
| 'RELEASE'
| 'RELOCATE'
| 'REMOVE_REGIONS'
| 'RENAME'
| 'REPEATABLE'
| 'REPLACE'
Expand Down Expand Up @@ -1394,7 +1395,6 @@ unreserved_keyword ::=
| 'SKIP_MISSING_SEQUENCE_OWNERS'
| 'SKIP_MISSING_VIEWS'
| 'SKIP_MISSING_UDFS'
| 'STRIP_LOCALITIES'
| 'SNAPSHOT'
| 'SPLIT'
| 'SQL'
Expand Down Expand Up @@ -2675,7 +2675,7 @@ restore_options ::=
| 'UNSAFE_RESTORE_INCOMPATIBLE_VERSION'
| 'EXECUTION' 'LOCALITY' '=' string_or_placeholder
| 'EXPERIMENTAL' 'DEFERRED' 'COPY'
| 'STRIP_LOCALITIES'
| 'REMOVE_REGIONS'

scrub_option_list ::=
( scrub_option ) ( ( ',' scrub_option ) )*
Expand Down Expand Up @@ -3918,6 +3918,7 @@ bare_label_keywords ::=
| 'RELATIVE'
| 'RELEASE'
| 'RELOCATE'
| 'REMOVE_REGIONS'
| 'RENAME'
| 'REPEATABLE'
| 'REPLACE'
Expand Down Expand Up @@ -3984,7 +3985,6 @@ bare_label_keywords ::=
| 'SKIP_MISSING_SEQUENCE_OWNERS'
| 'SKIP_MISSING_UDFS'
| 'SKIP_MISSING_VIEWS'
| 'STRIP_LOCALITIES'
| 'SMALLINT'
| 'SNAPSHOT'
| 'SOME'
Expand Down
4 changes: 2 additions & 2 deletions pkg/ccl/backupccl/restore_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -1053,7 +1053,7 @@ func createImportingDescriptors(
typesByID[types[i].GetID()] = types[i]
}

if details.StripLocalities {
if details.RemoveRegions {
// Can't restore multi-region tables into non-multi-region database
for _, t := range tables {
t.TableDesc().LocalityConfig = nil
Expand Down Expand Up @@ -1093,7 +1093,7 @@ func createImportingDescriptors(
// we need to make sure that multi-region databases no longer get tagged as such - meaning
// that we want to change the TypeDescriptor_MULTIREGION_ENUM to a normal enum. We `continue`
// to skip the multi-region work below.
if details.StripLocalities {
if details.RemoveRegions {
t.TypeDesc().Kind = descpb.TypeDescriptor_ENUM
t.TypeDesc().RegionConfig = nil
continue
Expand Down
4 changes: 2 additions & 2 deletions pkg/ccl/backupccl/restore_multiregion_rbr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (
// The goal of this test is to ensure that if a user ever performed a
// regionless restore where the backed-up target has a regional by row table,
// they would be able to get themselves out of a stuck state without needing
// an enterprise license (in addition to testing the ability to use strip_localities
// an enterprise license (in addition to testing the ability to use remove_regions
// without said license).
func TestMultiRegionRegionlessRestoreNoLicense(t *testing.T) {
defer leaktest.AfterTest(t)()
Expand Down Expand Up @@ -81,7 +81,7 @@ func TestMultiRegionRegionlessRestoreNoLicense(t *testing.T) {
defer sqlTC.Stopper().Stop(ctx)
sqlDB := sqlutils.MakeSQLRunner(sqlTC.Conns[0])

if err := backuptestutils.VerifyBackupRestoreStatementResult(t, sqlDB, `RESTORE DATABASE d FROM LATEST IN $1 WITH strip_localities`, localFoo); err != nil {
if err := backuptestutils.VerifyBackupRestoreStatementResult(t, sqlDB, `RESTORE DATABASE d FROM LATEST IN $1 WITH remove_regions`, localFoo); err != nil {
t.Fatal(err)
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/ccl/backupccl/restore_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -1081,7 +1081,7 @@ func resolveOptionsForRestoreJobDescription(
VerifyData: opts.VerifyData,
UnsafeRestoreIncompatibleVersion: opts.UnsafeRestoreIncompatibleVersion,
ExperimentalOnline: opts.ExperimentalOnline,
StripLocalities: opts.StripLocalities,
RemoveRegions: opts.RemoveRegions,
}

if opts.EncryptionPassphrase != nil {
Expand Down Expand Up @@ -2075,7 +2075,7 @@ func doRestorePlan(
// If we are stripping localities, wipe tables of their LocalityConfig before we allocate
// descriptor rewrites - as validation in remapTables compares these tables with the non-mr
// database and fails otherwise
if restoreStmt.Options.StripLocalities {
if restoreStmt.Options.RemoveRegions {
for _, t := range filteredTablesByID {
t.TableDesc().LocalityConfig = nil
}
Expand Down Expand Up @@ -2197,7 +2197,7 @@ func doRestorePlan(
SkipLocalitiesCheck: restoreStmt.Options.SkipLocalitiesCheck,
ExecutionLocality: execLocality,
ExperimentalOnline: restoreStmt.Options.ExperimentalOnline,
StripLocalities: restoreStmt.Options.StripLocalities,
RemoveRegions: restoreStmt.Options.RemoveRegions,
}

jr := jobs.Record{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ subtest end
new-cluster name=s2 share-io-dir=s1 allow-implicit-access localities=us-east-1
----

# test cluster restore with strip_localities
# test cluster restore with remove_regions
subtest restore_regionless_on_single_region_cluster

query-sql
Expand All @@ -56,7 +56,7 @@ postgres root <nil> <nil> {} <nil>
system node <nil> <nil> {} <nil>

exec-sql
RESTORE FROM LATEST IN 'nodelocal://1/cluster_backup/' WITH strip_localities;
RESTORE FROM LATEST IN 'nodelocal://1/cluster_backup/' WITH remove_regions;
----

# check cluster's regions
Expand Down Expand Up @@ -167,15 +167,15 @@ subtest end

subtest end

# test db restore with strip_localities
# test db restore with remove_regions
subtest restore_regionless_on_single_region_db

exec-sql
DROP DATABASE d;
----

exec-sql
RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/database_backup/' WITH strip_localities;
RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/database_backup/' WITH remove_regions;
----

# check to see if restored database, d, shows up
Expand Down Expand Up @@ -270,15 +270,15 @@ subtest end

subtest end

# test table restore with strip_localities
# test table restore with remove_regions
subtest restore_regionless_on_single_region_table

exec-sql
DROP TABLE d.t;
----

exec-sql
RESTORE TABLE d.t FROM LATEST IN 'nodelocal://1/table_backup/' WITH strip_localities;
RESTORE TABLE d.t FROM LATEST IN 'nodelocal://1/table_backup/' WITH remove_regions;
----

query-sql
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ subtest end
new-cluster name=s2 share-io-dir=s1 allow-implicit-access
----

# test cluster restore with strip_localities
# test cluster restore with remove_regions
subtest restore_regionless_on_regionless_cluster

query-sql
Expand All @@ -55,7 +55,7 @@ postgres root <nil> <nil> {} <nil>
system node <nil> <nil> {} <nil>

exec-sql
RESTORE FROM LATEST IN 'nodelocal://1/cluster_backup/' WITH strip_localities;
RESTORE FROM LATEST IN 'nodelocal://1/cluster_backup/' WITH remove_regions;
----

# check cluster's regions
Expand Down Expand Up @@ -100,15 +100,15 @@ SELECT * FROM d.t;

subtest end

# test db restore with strip_localities
# test db restore with remove_regions
subtest restore_regionless_on_regionless_db

exec-sql
DROP DATABASE d;
----

exec-sql
RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/database_backup/' WITH strip_localities;
RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/database_backup/' WITH remove_regions;
----

# check to see if restored database, d, shows up
Expand Down Expand Up @@ -148,15 +148,15 @@ SELECT * FROM d.t;

subtest end

# test table restore with strip_localities
# test table restore with remove_regions
subtest restore_regionless_on_regionless_table

exec-sql
DROP TABLE d.t;
----

exec-sql
RESTORE TABLE d.t FROM LATEST IN 'nodelocal://1/table_backup/' WITH strip_localities;
RESTORE TABLE d.t FROM LATEST IN 'nodelocal://1/table_backup/' WITH remove_regions;
----

query-sql
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ new-cluster name=s2 share-io-dir=s1 allow-implicit-access localities=us-east-1
subtest restore_regionless_cluster_rbr

exec-sql
RESTORE FROM LATEST IN 'nodelocal://1/rbr_cluster_backup/' WITH strip_localities;
RESTORE FROM LATEST IN 'nodelocal://1/rbr_cluster_backup/' WITH remove_regions;
----

# check cluster's regions
Expand Down Expand Up @@ -129,7 +129,7 @@ DROP DATABASE d;
----

exec-sql
RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/rbr_database_backup/' WITH strip_localities;
RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/rbr_database_backup/' WITH remove_regions;
----

# check to see if restored database, d, shows up
Expand Down Expand Up @@ -191,7 +191,7 @@ DROP TABLE d.t;
----

exec-sql
RESTORE TABLE d.t FROM LATEST IN 'nodelocal://1/rbr_table_backup/' WITH strip_localities;
RESTORE TABLE d.t FROM LATEST IN 'nodelocal://1/rbr_table_backup/' WITH remove_regions;
----
pq: "crdb_internal_region" is not compatible with type "crdb_internal_region" existing in cluster: "crdb_internal_region" of type "ENUM" is not compatible with type "MULTIREGION_ENUM"

Expand All @@ -201,7 +201,7 @@ DROP TYPE d.public.crdb_internal_region;
----

exec-sql
RESTORE TABLE d.t FROM LATEST IN 'nodelocal://1/rbr_table_backup/' WITH strip_localities;
RESTORE TABLE d.t FROM LATEST IN 'nodelocal://1/rbr_table_backup/' WITH remove_regions;
----

query-sql
Expand Down
5 changes: 5 additions & 0 deletions pkg/ccl/streamingccl/streamingest/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ go_library(
"ingest_span_configs.go",
"merged_subscription.go",
"metrics.go",
"replication_execution_details.go",
"stream_ingest_manager.go",
"stream_ingestion_dist.go",
"stream_ingestion_frontier_processor.go",
Expand Down Expand Up @@ -67,6 +68,7 @@ go_library(
"//pkg/util/bulk",
"//pkg/util/ctxgroup",
"//pkg/util/hlc",
"//pkg/util/humanizeutil",
"//pkg/util/log",
"//pkg/util/metric",
"//pkg/util/protoutil",
Expand All @@ -92,6 +94,7 @@ go_test(
"main_test.go",
"merged_subscription_test.go",
"rangekey_batcher_test.go",
"replication_execution_details_test.go",
"replication_random_client_test.go",
"replication_stream_e2e_test.go",
"stream_ingestion_dist_test.go",
Expand Down Expand Up @@ -134,6 +137,7 @@ go_test(
"//pkg/security/securitytest",
"//pkg/security/username",
"//pkg/server",
"//pkg/server/serverpb",
"//pkg/settings",
"//pkg/settings/cluster",
"//pkg/spanconfig",
Expand All @@ -159,6 +163,7 @@ go_test(
"//pkg/testutils/testcluster",
"//pkg/util/ctxgroup",
"//pkg/util/hlc",
"//pkg/util/httputil",
"//pkg/util/leaktest",
"//pkg/util/limit",
"//pkg/util/log",
Expand Down
5 changes: 5 additions & 0 deletions pkg/ccl/streamingccl/streamingest/datadriven_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ func TestDataDriven(t *testing.T) {

ctx := context.Background()
datadriven.Walk(t, datapathutils.TestDataPath(t), func(t *testing.T, path string) {
// Skip the test if it is a .txt file. This is to allow us to have non-test
// testdata in the same directory as the test files.
if strings.HasSuffix(path, ".txt") {
return
}
ds := newDatadrivenTestState()
defer ds.cleanup(t)
datadriven.RunTest(t, path, func(t *testing.T, d *datadriven.TestData) string {
Expand Down
Loading

0 comments on commit 2ac6153

Please sign in to comment.