From bc864578102bc4701aa9d3b145b1ee984f172d78 Mon Sep 17 00:00:00 2001 From: Rafi Shamim Date: Tue, 26 Mar 2024 16:06:12 -0400 Subject: [PATCH 1/2] backupccl: stop relying on fixed IDs in datadriven tests The IDs of objects created in this test are not deterministic between runs. If a transaction auto-retries while creating an object, an ID can be skipped. Release note: None --- .../backup-restore/backup-dropped-descriptors | 26 ++++++++++++++++--- .../backup-dropped-descriptors-declarative | 26 ++++++++++++++++--- .../backup-restore/plpgsql_procedures | 10 +++---- .../plpgsql_user_defined_functions | 10 +++---- .../testdata/backup-restore/procedures | 10 +++---- .../testdata/backup-restore/regression-tests | 14 +++------- .../backup-restore/user-defined-functions | 10 +++---- 7 files changed, 66 insertions(+), 40 deletions(-) diff --git a/pkg/ccl/backupccl/testdata/backup-restore/backup-dropped-descriptors b/pkg/ccl/backupccl/testdata/backup-restore/backup-dropped-descriptors index 732feedcf3b0..a04b2bad1752 100644 --- a/pkg/ccl/backupccl/testdata/backup-restore/backup-dropped-descriptors +++ b/pkg/ccl/backupccl/testdata/backup-restore/backup-dropped-descriptors @@ -18,6 +18,10 @@ CREATE DATABASE d; CREATE TABLE d.foo (id INT); ---- +let $d_id +SELECT id FROM system.namespace WHERE name = 'd' AND "parentID" = 0; +---- + schema-change expect-pausepoint DROP DATABASE d CASCADE; ---- @@ -28,7 +32,7 @@ query-sql WITH tbls AS ( SELECT id, crdb_internal.pb_to_json('cockroach.sql.sqlbase.Descriptor', descriptor) AS orig FROM system.descriptor ) -SELECT orig->'database'->'name', orig->'database'->'state' FROM tbls WHERE id = 107; +SELECT orig->'database'->'name', orig->'database'->'state' FROM tbls WHERE id = $d_id; ---- "d" "DROP" @@ -122,6 +126,22 @@ CREATE SCHEMA d2.s; CREATE TABLE d2.s.t (id INT); ---- +let $d2_id +SELECT id FROM system.namespace WHERE name = 'd2' AND "parentID" = 0; +---- + +let $s_id +SELECT id FROM system.namespace WHERE name = 's' AND "parentID" = $d2_id; +---- + +let $typ_id +SELECT id FROM system.namespace WHERE name = 'typ' AND "parentID" = $d2_id; +---- + +let $typArray_id +SELECT id FROM system.namespace WHERE name = '_typ' AND "parentID" = $d2_id; +---- + exec-sql SET use_declarative_schema_changer = 'off'; ---- @@ -148,7 +168,7 @@ query-sql WITH tbls AS ( SELECT id, crdb_internal.pb_to_json('cockroach.sql.sqlbase.Descriptor', descriptor) AS orig FROM system.descriptor ) -SELECT orig->'schema'->'name', orig->'schema'->'state' FROM tbls WHERE id = 112; +SELECT orig->'schema'->'name', orig->'schema'->'state' FROM tbls WHERE id = $s_id; ---- "s" "DROP" @@ -157,7 +177,7 @@ query-sql WITH tbls AS ( SELECT id, crdb_internal.pb_to_json('cockroach.sql.sqlbase.Descriptor', descriptor) AS orig FROM system.descriptor ) -SELECT orig->'type'->'name', orig->'type'->'state' FROM tbls WHERE id = 110 OR id = 111; +SELECT orig->'type'->'name', orig->'type'->'state' FROM tbls WHERE id = $typ_id OR id = $typArray_id; ---- "typ" "DROP" "_typ" "DROP" diff --git a/pkg/ccl/backupccl/testdata/backup-restore/backup-dropped-descriptors-declarative b/pkg/ccl/backupccl/testdata/backup-restore/backup-dropped-descriptors-declarative index 2a41a410d2ce..6c8a7bf9c707 100644 --- a/pkg/ccl/backupccl/testdata/backup-restore/backup-dropped-descriptors-declarative +++ b/pkg/ccl/backupccl/testdata/backup-restore/backup-dropped-descriptors-declarative @@ -19,6 +19,10 @@ CREATE TABLE dd.foo (id INT); CREATE SCHEMA dd.s; ---- +let $dd_id +SELECT id FROM system.namespace WHERE name = 'dd' AND "parentID" = 0; +---- + new-schema-change expect-pausepoint DROP DATABASE dd CASCADE; ---- @@ -29,7 +33,7 @@ query-sql WITH tbls AS ( SELECT id, crdb_internal.pb_to_json('cockroach.sql.sqlbase.Descriptor', descriptor) AS orig FROM system.descriptor ) -SELECT orig->'database'->'name', orig->'database'->'state' FROM tbls WHERE id = 107; +SELECT orig->'database'->'name', orig->'database'->'state' FROM tbls WHERE id = $dd_id; ---- "dd" "DROP" @@ -75,6 +79,22 @@ CREATE SCHEMA d2.s; CREATE TABLE d2.s.t (id INT); ---- +let $d2_id +SELECT id FROM system.namespace WHERE name = 'd2' AND "parentID" = 0; +---- + +let $s_id +SELECT id FROM system.namespace WHERE name = 's' AND "parentID" = $d2_id; +---- + +let $typ_id +SELECT id FROM system.namespace WHERE name = 'typ' AND "parentID" = $d2_id; +---- + +let $typArray_id +SELECT id FROM system.namespace WHERE name = '_typ' AND "parentID" = $d2_id; +---- + exec-sql SET use_declarative_schema_changer = 'on'; ---- @@ -97,7 +117,7 @@ query-sql WITH tbls AS ( SELECT id, crdb_internal.pb_to_json('cockroach.sql.sqlbase.Descriptor', descriptor) AS orig FROM system.descriptor ) -SELECT orig->'schema'->'name', orig->'schema'->'state' FROM tbls WHERE id = 112; +SELECT orig->'schema'->'name', orig->'schema'->'state' FROM tbls WHERE id = $s_id; ---- "s" "DROP" @@ -105,7 +125,7 @@ query-sql WITH tbls AS ( SELECT id, crdb_internal.pb_to_json('cockroach.sql.sqlbase.Descriptor', descriptor) AS orig FROM system.descriptor ) -SELECT orig->'type'->'name', orig->'type'->'state' FROM tbls WHERE id = 110 OR id = 111; +SELECT orig->'type'->'name', orig->'type'->'state' FROM tbls WHERE id = $typ_id OR id = $typArray_id; ---- "typ" "DROP" "_typ" "DROP" diff --git a/pkg/ccl/backupccl/testdata/backup-restore/plpgsql_procedures b/pkg/ccl/backupccl/testdata/backup-restore/plpgsql_procedures index 5191b59d865f..93b259505728 100644 --- a/pkg/ccl/backupccl/testdata/backup-restore/plpgsql_procedures +++ b/pkg/ccl/backupccl/testdata/backup-restore/plpgsql_procedures @@ -353,7 +353,7 @@ CREATE PROCEDURE sc1.p() LANGUAGE PLpgSQL AS $$ BEGIN SELECT 1; END $$; ---- # Make sure the original schema has procedure signatures -query-sql +let $defaultdb_sc1_id WITH db_id AS ( SELECT id FROM system.namespace WHERE name = 'defaultdb' ), @@ -365,7 +365,6 @@ schema_id AS ( ) SELECT id FROM schema_id; ---- -109 query-sql WITH to_json AS ( @@ -378,7 +377,7 @@ WITH to_json AS ( ) AS d FROM system.descriptor - WHERE id = 109 + WHERE id = $defaultdb_sc1_id ) SELECT d->'schema'->>'functions'::string FROM to_json; ---- @@ -396,7 +395,7 @@ exec-sql USE db1; ---- -query-sql +let $db1_sc1_id WITH db_id AS ( SELECT id FROM system.namespace WHERE name = 'db1' ), @@ -408,7 +407,6 @@ schema_id AS ( ) SELECT id FROM schema_id; ---- -112 query-sql WITH to_json AS ( @@ -421,7 +419,7 @@ WITH to_json AS ( ) AS d FROM system.descriptor - WHERE id = 112 + WHERE id = $db1_sc1_id ) SELECT d->'schema'->>'functions'::string FROM to_json; ---- diff --git a/pkg/ccl/backupccl/testdata/backup-restore/plpgsql_user_defined_functions b/pkg/ccl/backupccl/testdata/backup-restore/plpgsql_user_defined_functions index 445377d5a8c8..92eef8ee1fd9 100644 --- a/pkg/ccl/backupccl/testdata/backup-restore/plpgsql_user_defined_functions +++ b/pkg/ccl/backupccl/testdata/backup-restore/plpgsql_user_defined_functions @@ -379,7 +379,7 @@ CREATE FUNCTION sc1.f() RETURNS INT LANGUAGE PLpgSQL AS $$ BEGIN RETURN 1; END $ ---- # Make sure the original schema has function signatures -query-sql +let $defaultdb_sc1_db WITH db_id AS ( SELECT id FROM system.namespace WHERE name = 'defaultdb' ), @@ -391,7 +391,6 @@ schema_id AS ( ) SELECT id FROM schema_id; ---- -109 query-sql WITH to_json AS ( @@ -404,7 +403,7 @@ WITH to_json AS ( ) AS d FROM system.descriptor - WHERE id = 109 + WHERE id = $defaultdb_sc1_db ) SELECT d->'schema'->>'functions'::string FROM to_json; ---- @@ -422,7 +421,7 @@ exec-sql USE db1; ---- -query-sql +let $db1_sc1_id WITH db_id AS ( SELECT id FROM system.namespace WHERE name = 'db1' ), @@ -434,7 +433,6 @@ schema_id AS ( ) SELECT id FROM schema_id; ---- -112 query-sql WITH to_json AS ( @@ -447,7 +445,7 @@ WITH to_json AS ( ) AS d FROM system.descriptor - WHERE id = 112 + WHERE id = $db1_sc1_id ) SELECT d->'schema'->>'functions'::string FROM to_json; ---- diff --git a/pkg/ccl/backupccl/testdata/backup-restore/procedures b/pkg/ccl/backupccl/testdata/backup-restore/procedures index 89634cf91221..d0dad064130f 100644 --- a/pkg/ccl/backupccl/testdata/backup-restore/procedures +++ b/pkg/ccl/backupccl/testdata/backup-restore/procedures @@ -325,7 +325,7 @@ CREATE PROCEDURE sc1.p() LANGUAGE SQL AS $$ SELECT 1 $$; ---- # Make sure the original schema has procedure signatures -query-sql +let $defaultdb_sc1_db WITH db_id AS ( SELECT id FROM system.namespace WHERE name = 'defaultdb' ), @@ -337,7 +337,6 @@ schema_id AS ( ) SELECT id FROM schema_id; ---- -109 query-sql WITH to_json AS ( @@ -350,7 +349,7 @@ WITH to_json AS ( ) AS d FROM system.descriptor - WHERE id = 109 + WHERE id = $defaultdb_sc1_db ) SELECT d->'schema'->>'functions'::string FROM to_json; ---- @@ -368,7 +367,7 @@ exec-sql USE db1; ---- -query-sql +let $db1_sc1_id WITH db_id AS ( SELECT id FROM system.namespace WHERE name = 'db1' ), @@ -380,7 +379,6 @@ schema_id AS ( ) SELECT id FROM schema_id; ---- -112 query-sql WITH to_json AS ( @@ -393,7 +391,7 @@ WITH to_json AS ( ) AS d FROM system.descriptor - WHERE id = 112 + WHERE id = $db1_sc1_id ) SELECT d->'schema'->>'functions'::string FROM to_json; ---- diff --git a/pkg/ccl/backupccl/testdata/backup-restore/regression-tests b/pkg/ccl/backupccl/testdata/backup-restore/regression-tests index 246b124d6713..4963f39c8986 100644 --- a/pkg/ccl/backupccl/testdata/backup-restore/regression-tests +++ b/pkg/ccl/backupccl/testdata/backup-restore/regression-tests @@ -183,6 +183,8 @@ exec-sql CREATE DATABASE b; ---- +# The ID of "me" is normally 109. However, we can't add an assertion for that since ID +# generation is not deterministic. exec-sql CREATE SCHEMA b.me; ---- @@ -199,11 +201,6 @@ exec-sql BACKUP INTO 'nodelocal://1/cluster' ---- -query-sql -SELECT id, name FROM system.namespace WHERE name = 'me'; ----- -109 me - new-cluster name=s5 share-io-dir=s4 ---- @@ -217,15 +214,12 @@ CREATE SCHEMA d.bar; CREATE SCHEMA d.baz; ---- +# The ID of "collide" is normally 109. However, we can't add an assertion for that since ID +# generation is not deterministic. exec-sql CREATE DATABASE collide ---- -query-sql -SELECT id, name FROM system.namespace WHERE name = 'collide' ----- -109 collide - exec-sql RESTORE TABLE b.me.foo FROM LATEST IN 'nodelocal://1/cluster' WITH into_db=collide ---- diff --git a/pkg/ccl/backupccl/testdata/backup-restore/user-defined-functions b/pkg/ccl/backupccl/testdata/backup-restore/user-defined-functions index 174433348d24..c734d4f71ddd 100644 --- a/pkg/ccl/backupccl/testdata/backup-restore/user-defined-functions +++ b/pkg/ccl/backupccl/testdata/backup-restore/user-defined-functions @@ -321,7 +321,7 @@ CREATE FUNCTION sc1.f() RETURNS INT LANGUAGE SQL AS $$ SELECT 1 $$; ---- # Make sure the original schema has function signatures -query-sql +let $defaultdb_sc1_db WITH db_id AS ( SELECT id FROM system.namespace WHERE name = 'defaultdb' ), @@ -333,7 +333,6 @@ schema_id AS ( ) SELECT id FROM schema_id; ---- -109 query-sql WITH to_json AS ( @@ -346,7 +345,7 @@ WITH to_json AS ( ) AS d FROM system.descriptor - WHERE id = 109 + WHERE id = $defaultdb_sc1_db ) SELECT d->'schema'->>'functions'::string FROM to_json; ---- @@ -364,7 +363,7 @@ exec-sql USE db1; ---- -query-sql +let $db1_sc1_id WITH db_id AS ( SELECT id FROM system.namespace WHERE name = 'db1' ), @@ -376,7 +375,6 @@ schema_id AS ( ) SELECT id FROM schema_id; ---- -112 query-sql WITH to_json AS ( @@ -389,7 +387,7 @@ WITH to_json AS ( ) AS d FROM system.descriptor - WHERE id = 112 + WHERE id = $db1_sc1_id ) SELECT d->'schema'->>'functions'::string FROM to_json; ---- From a90eb16376d2ff0b5e777eac1da872d06ff23227 Mon Sep 17 00:00:00 2001 From: Jackson Owens Date: Tue, 26 Mar 2024 20:41:23 -0400 Subject: [PATCH 2/2] storage: require enterprise license for WAL failover If WAL failover is configured but the user has not provided an enterprise license, WAL failover will refuse to failover and log a warning message every 10 minutes. Epic: none Release note: none --- pkg/storage/open.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/pkg/storage/open.go b/pkg/storage/open.go index fead3679e041..e9f37eec6745 100644 --- a/pkg/storage/open.go +++ b/pkg/storage/open.go @@ -22,6 +22,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/storage/fs" + "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/errors" "github.com/cockroachdb/pebble" "github.com/cockroachdb/pebble/vfs" @@ -394,6 +395,7 @@ func WALFailover(walCfg base.WALFailoverConfig, storeEnvs fs.Envs) ConfigOption func makePebbleWALFailoverOptsForDir( settings *cluster.Settings, dir wal.Dir, ) *pebble.WALFailoverOptions { + cclWALFailoverLogEvery := log.Every(10 * time.Minute) return &pebble.WALFailoverOptions{ Secondary: dir, FailoverOptions: wal.FailoverOptions{ @@ -408,8 +410,13 @@ func makePebbleWALFailoverOptsForDir( // // NB: We do not use settings.Version.IsActive because we do not have a // guarantee that the cluster version has been initialized. - failoverOK := settings.Version.ActiveVersionOrEmpty(context.TODO()).IsActive(clusterversion.V24_1Start) - return walFailoverUnhealthyOpThreshold.Get(&settings.SV), failoverOK + versionOK := settings.Version.ActiveVersionOrEmpty(context.TODO()).IsActive(clusterversion.V24_1Start) + // WAL failover is a licensed feature. + licenseOK := base.CCLDistributionAndEnterpriseEnabled(settings) + if !licenseOK && cclWALFailoverLogEvery.ShouldLog() { + log.Warningf(context.Background(), "Ignoring WAL failover configuration because it requires an enterprise license.") + } + return walFailoverUnhealthyOpThreshold.Get(&settings.SV), versionOK && licenseOK }, }, }