Skip to content

Commit

Permalink
server,db-console: add feature flag for Cross-Cluster Replication das…
Browse files Browse the repository at this point in the history
…hboard

This change adds a feature flag whose value is determined by the cluster
setting `cross_cluster_replication.enabled`. This feature flag is then used
when rendering the metrics dashboard dropdown options to decide whether to
display the cross-cluster replication dashboard.

This change also deletes the session variable and associated cluster setting
`sql.defaults.experimental_stream_replication.enabled` so as to unify all
interactions with cross-cluster replication under a single cluster setting.

Fixes: #95995

Release note: None
  • Loading branch information
adityamaru committed Feb 1, 2023
1 parent 79f6e5b commit 5cde7e2
Show file tree
Hide file tree
Showing 19 changed files with 75 additions and 45 deletions.
3 changes: 0 additions & 3 deletions docs/generated/settings/settings-for-tenants.txt
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,6 @@ This session variable default should now be configured using ALTER ROLE... SET:
sql.defaults.experimental_implicit_column_partitioning.enabled boolean false "default value for experimental_enable_temp_tables; allows for the use of implicit column partitioning
This cluster setting is being kept to preserve backwards-compatibility.
This session variable default should now be configured using ALTER ROLE... SET: https://www.cockroachlabs.com/docs/stable/alter-role.html"
sql.defaults.experimental_stream_replication.enabled boolean false "default value for experimental_stream_replication session setting;enables the ability to setup a replication stream
This cluster setting is being kept to preserve backwards-compatibility.
This session variable default should now be configured using ALTER ROLE... SET: https://www.cockroachlabs.com/docs/stable/alter-role.html"
sql.defaults.experimental_temporary_tables.enabled boolean false "default value for experimental_enable_temp_tables; allows for use of temporary tables by default
This cluster setting is being kept to preserve backwards-compatibility.
This session variable default should now be configured using ALTER ROLE... SET: https://www.cockroachlabs.com/docs/stable/alter-role.html"
Expand Down
1 change: 0 additions & 1 deletion docs/generated/settings/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@
<tr><td><div id="setting-sql-defaults-experimental-distsql-planning" class="anchored"><code>sql.defaults.experimental_distsql_planning</code></div></td><td>enumeration</td><td><code>off</code></td><td>default experimental_distsql_planning mode; enables experimental opt-driven DistSQL planning [off = 0, on = 1]<br/>This cluster setting is being kept to preserve backwards-compatibility.<br/>This session variable default should now be configured using <a href="alter-role.html"><code>ALTER ROLE... SET</code></a></td></tr>
<tr><td><div id="setting-sql-defaults-experimental-enable-unique-without-index-constraints-enabled" class="anchored"><code>sql.defaults.experimental_enable_unique_without_index_constraints.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>default value for experimental_enable_unique_without_index_constraints session setting;disables unique without index constraints by default<br/>This cluster setting is being kept to preserve backwards-compatibility.<br/>This session variable default should now be configured using <a href="alter-role.html"><code>ALTER ROLE... SET</code></a></td></tr>
<tr><td><div id="setting-sql-defaults-experimental-implicit-column-partitioning-enabled" class="anchored"><code>sql.defaults.experimental_implicit_column_partitioning.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>default value for experimental_enable_temp_tables; allows for the use of implicit column partitioning<br/>This cluster setting is being kept to preserve backwards-compatibility.<br/>This session variable default should now be configured using <a href="alter-role.html"><code>ALTER ROLE... SET</code></a></td></tr>
<tr><td><div id="setting-sql-defaults-experimental-stream-replication-enabled" class="anchored"><code>sql.defaults.experimental_stream_replication.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>default value for experimental_stream_replication session setting;enables the ability to setup a replication stream<br/>This cluster setting is being kept to preserve backwards-compatibility.<br/>This session variable default should now be configured using <a href="alter-role.html"><code>ALTER ROLE... SET</code></a></td></tr>
<tr><td><div id="setting-sql-defaults-experimental-temporary-tables-enabled" class="anchored"><code>sql.defaults.experimental_temporary_tables.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>default value for experimental_enable_temp_tables; allows for use of temporary tables by default<br/>This cluster setting is being kept to preserve backwards-compatibility.<br/>This session variable default should now be configured using <a href="alter-role.html"><code>ALTER ROLE... SET</code></a></td></tr>
<tr><td><div id="setting-sql-defaults-foreign-key-cascades-limit" class="anchored"><code>sql.defaults.foreign_key_cascades_limit</code></div></td><td>integer</td><td><code>10000</code></td><td>default value for foreign_key_cascades_limit session setting; limits the number of cascading operations that run as part of a single query<br/>This cluster setting is being kept to preserve backwards-compatibility.<br/>This session variable default should now be configured using <a href="alter-role.html"><code>ALTER ROLE... SET</code></a></td></tr>
<tr><td><div id="setting-sql-defaults-idle-in-session-timeout" class="anchored"><code>sql.defaults.idle_in_session_timeout</code></div></td><td>duration</td><td><code>0s</code></td><td>default value for the idle_in_session_timeout; default value for the idle_in_session_timeout session setting; controls the duration a session is permitted to idle before the session is terminated; if set to 0, there is no timeout<br/>This cluster setting is being kept to preserve backwards-compatibility.<br/>This session variable default should now be configured using <a href="alter-role.html"><code>ALTER ROLE... SET</code></a></td></tr>
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/streamingccl/replicationtestutils/testutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ func CreateTenantStreamingClusters(
args.DestInitFunc(t, tsc.DestSysSQL)
}
// Enable stream replication on dest by default.
tsc.DestSysSQL.Exec(t, `SET enable_experimental_stream_replication = true;`)
tsc.DestSysSQL.Exec(t, `SET CLUSTER SETTING cross_cluster_replication.enabled = true;`)
return tsc, func() {
require.NoError(t, srcTenantConn.Close())
destCleanup()
Expand Down
9 changes: 9 additions & 0 deletions pkg/ccl/streamingccl/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@ import (
"github.com/cockroachdb/cockroach/pkg/settings"
)

// CrossClusterReplicationEnabled enables the ability to setup and control a
// cross cluster replication stream.
var CrossClusterReplicationEnabled = settings.RegisterBoolSetting(
settings.TenantWritable,
"cross_cluster_replication.enabled",
"enables the ability to setup and control a cross cluster replication stream",
false,
)

// StreamReplicationStreamLivenessTrackFrequency controls frequency to check
// the liveness of a streaming replication producer job.
var StreamReplicationStreamLivenessTrackFrequency = settings.RegisterDurationSetting(
Expand Down
14 changes: 14 additions & 0 deletions pkg/ccl/streamingccl/streamingest/alter_replication_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"context"
"math"

"github.com/cockroachdb/cockroach/pkg/ccl/streamingccl"
"github.com/cockroachdb/cockroach/pkg/ccl/streamingccl/replicationutils"
"github.com/cockroachdb/cockroach/pkg/ccl/utilccl"
"github.com/cockroachdb/cockroach/pkg/jobs"
Expand Down Expand Up @@ -111,6 +112,19 @@ func alterReplicationJobHook(
return nil, nil, nil, false, nil
}

if !streamingccl.CrossClusterReplicationEnabled.Get(&p.ExecCfg().Settings.SV) {
return nil, nil, nil, false, errors.WithTelemetry(
pgerror.WithCandidateCode(
errors.WithHint(
errors.Newf("cross cluster replication is disabled"),
"You can enable cross cluster replication by running `SET CLUSTER SETTING cross_cluster_replication.enabled = true`.",
),
pgcode.ExperimentalFeature,
),
"cross_cluster_replication.enabled",
)
}

if err := p.RequireAdminRole(ctx, "ALTER TENANT REPLICATION"); err != nil {
return nil, nil, nil, false, err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ func TestStreamIngestionJobWithRandomClient(t *testing.T) {
_, err = conn.Exec(query)
require.True(t, testutils.IsError(err, "stream replication is only supported experimentally"))

_, err = conn.Exec(`SET enable_experimental_stream_replication = true`)
_, err = conn.Exec(`SET CLUSTER SETTING cross_cluster_replication.enabled = true;`)
require.NoError(t, err)

_, err = conn.Exec(query)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ SET CLUSTER SETTING stream_replication.consumer_heartbeat_frequency = '100ms';
SET CLUSTER SETTING bulkio.stream_ingestion.minimum_flush_interval = '500ms';
SET CLUSTER SETTING bulkio.stream_ingestion.cutover_signal_poll_interval = '100ms';
SET CLUSTER SETTING stream_replication.job_checkpoint_frequency = '100ms';
SET enable_experimental_stream_replication = true;
SET CLUSTER SETTING cross_cluster_replication.enabled = true;
`,
";")...)

Expand Down Expand Up @@ -191,7 +191,7 @@ func TestTenantStreamingCreationErrors(t *testing.T) {
SrcSysSQL.Exec(t, `SET CLUSTER SETTING kv.rangefeed.enabled = true`)

DestSysSQL := sqlutils.MakeSQLRunner(destDB)
DestSysSQL.Exec(t, `SET enable_experimental_stream_replication = true`)
DestSysSQL.Exec(t, `SET CLUSTER SETTING cross_cluster_replication.enabled = true;`)

// Sink to read data from.
srcPgURL, cleanupSink := sqlutils.PGUrl(t, srcServer.ServingSQLAddr(), t.Name(), url.User(username.RootUser))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,17 +68,16 @@ func ingestionPlanHook(
return nil, nil, nil, false, nil
}

// Check if the experimental feature is enabled.
if !p.SessionData().EnableStreamReplication {
if !streamingccl.CrossClusterReplicationEnabled.Get(&p.ExecCfg().Settings.SV) {
return nil, nil, nil, false, errors.WithTelemetry(
pgerror.WithCandidateCode(
errors.WithHint(
errors.Newf("stream replication is only supported experimentally"),
"You can enable stream replication by running `SET enable_experimental_stream_replication = true`.",
errors.Newf("cross cluster replication is disabled"),
"You can enable cross cluster replication by running `SET CLUSTER SETTING cross_cluster_replication.enabled = true`.",
),
pgcode.ExperimentalFeature,
),
"replication.ingest.disabled",
"cross_cluster_replication.enabled",
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func startReplication(
conn, err := pgx.ConnectConfig(queryCtx, pgxConfig)
require.NoError(t, err)

rows, err := conn.Query(queryCtx, `SET enable_experimental_stream_replication = true`)
rows, err := conn.Query(queryCtx, `SET CLUSTER SETTING cross_cluster_replication.enabled = true;`)
require.NoError(t, err)
rows.Close()

Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/roachtest/tests/cluster_to_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ func srcClusterSettings(t test.Test, db *sqlutils.SQLRunner) {
}

func destClusterSettings(t test.Test, db *sqlutils.SQLRunner) {
db.ExecMultiple(t, `SET enable_experimental_stream_replication = true;`)
db.ExecMultiple(t, `SET CLUSTER SETTING cross_cluster_replication.enabled = true;`)
}

func copyPGCertsAndMakeURL(
Expand Down
1 change: 1 addition & 0 deletions pkg/server/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ go_library(
"//pkg/blobs",
"//pkg/blobs/blobspb",
"//pkg/build",
"//pkg/ccl/streamingccl",
"//pkg/cloud",
"//pkg/cloud/cloudpb",
"//pkg/cloud/externalconn",
Expand Down
8 changes: 8 additions & 0 deletions pkg/server/server_http.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ import (

"github.com/NYTimes/gziphandler"
"github.com/cockroachdb/cmux"
"github.com/cockroachdb/cockroach/pkg/ccl/streamingccl"
"github.com/cockroachdb/cockroach/pkg/rpc"
"github.com/cockroachdb/cockroach/pkg/server/debug"
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
"github.com/cockroachdb/cockroach/pkg/server/status"
"github.com/cockroachdb/cockroach/pkg/settings"
"github.com/cockroachdb/cockroach/pkg/ts"
Expand Down Expand Up @@ -116,6 +118,12 @@ func (s *httpServer) setupRoutes(
}
return nil
},
Flags: serverpb.FeatureFlags{
CrossClusterReplicationEnabled: func() bool {
return streamingccl.CrossClusterReplicationEnabled.Get(&s.cfg.Settings.SV)
}(),
},
Settings: s.cfg.Settings,
})

// The authentication mux used here is created in "allow anonymous" mode so that the UI
Expand Down
5 changes: 5 additions & 0 deletions pkg/server/serverpb/admin.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1444,4 +1444,9 @@ message SetTraceRecordingTypeResponse{}
message FeatureFlags {
// Whether the server is an instance of the Observability Service
bool is_observability_service = 1;

// CrossClusterReplicationEnabled gets its value from the cluster setting
// `cross_cluster_replication.enabled`. It controls whether or not we should
// show the Cross-Cluster Replication metrics page on the DB console.
bool cross_cluster_replication_enabled = 2;
}
4 changes: 2 additions & 2 deletions pkg/settings/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,8 @@ var retiredSettings = map[string]struct{}{
"changefeed.active_protected_timestamps.enabled": {},
"jobs.scheduler.single_node_scheduler.enabled": {},
// renamed.
"spanconfig.host_coalesce_adjacent.enabled": {},
"spanconfig.host_coalesce_adjacent.enabled": {},
"sql.defaults.experimental_stream_replication.enabled": {},
}

// sqlDefaultSettings is the list of "grandfathered" existing sql.defaults
Expand All @@ -182,7 +183,6 @@ var sqlDefaultSettings = map[string]struct{}{
"sql.defaults.experimental_distsql_planning": {},
"sql.defaults.experimental_enable_unique_without_index_constraints.enabled": {},
"sql.defaults.experimental_implicit_column_partitioning.enabled": {},
"sql.defaults.experimental_stream_replication.enabled": {},
"sql.defaults.experimental_temporary_tables.enabled": {},
"sql.defaults.foreign_key_cascades_limit": {},
"sql.defaults.idle_in_session_timeout": {},
Expand Down
8 changes: 0 additions & 8 deletions pkg/sql/exec_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -511,14 +511,6 @@ var experimentalUseNewSchemaChanger = settings.RegisterEnumSetting(
},
).WithPublic()

var experimentalStreamReplicationEnabled = settings.RegisterBoolSetting(
settings.TenantWritable,
"sql.defaults.experimental_stream_replication.enabled",
"default value for experimental_stream_replication session setting;"+
"enables the ability to setup a replication stream",
false,
).WithPublic()

var stubCatalogTablesEnabledClusterValue = settings.RegisterBoolSetting(
settings.TenantWritable,
`sql.defaults.stub_catalog_tables.enabled`,
Expand Down
18 changes: 0 additions & 18 deletions pkg/sql/vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -1745,24 +1745,6 @@ var varGen = map[string]sessionVar{
},
},

`enable_experimental_stream_replication`: {
GetStringVal: makePostgresBoolGetStringValFn(`enable_experimental_stream_replication`),
Set: func(_ context.Context, m sessionDataMutator, s string) error {
b, err := paramparse.ParseBoolVar(`enable_experimental_stream_replication`, s)
if err != nil {
return err
}
m.SetStreamReplicationEnabled(b)
return nil
},
Get: func(evalCtx *extendedEvalContext, _ *kv.Txn) (string, error) {
return formatBoolAsPostgresSetting(evalCtx.SessionData().EnableStreamReplication), nil
},
GlobalDefault: func(sv *settings.Values) string {
return formatBoolAsPostgresSetting(experimentalStreamReplicationEnabled.Get(sv))
},
},

// CockroachDB extension. See experimentalComputedColumnRewrites or
// ParseComputedColumnRewrites for a description of the format.
`experimental_computed_column_rewrites`: {
Expand Down
2 changes: 2 additions & 0 deletions pkg/ui/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ go_library(
deps = [
"//pkg/base",
"//pkg/build",
"//pkg/ccl/streamingccl",
"//pkg/server/serverpb",
"//pkg/settings/cluster",
"//pkg/util/httputil",
"//pkg/util/log",
],
Expand Down
14 changes: 13 additions & 1 deletion pkg/ui/ui.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ import (

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/build"
"github.com/cockroachdb/cockroach/pkg/ccl/streamingccl"
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/util/httputil"
"github.com/cockroachdb/cockroach/pkg/util/log"
)
Expand Down Expand Up @@ -104,6 +106,16 @@ type Config struct {
GetUser func(ctx context.Context) *string
OIDC OIDCUI
Flags serverpb.FeatureFlags
Settings *cluster.Settings
}

// GetFeatureFlags loads the feature flags for the Config. Some flags may be
// dynamically loaded to capture their most up-to-date values.
func (cfg Config) GetFeatureFlags() serverpb.FeatureFlags {
return serverpb.FeatureFlags{
CrossClusterReplicationEnabled: streamingccl.CrossClusterReplicationEnabled.Get(&cfg.Settings.SV),
IsObservabilityService: cfg.Flags.IsObservabilityService,
}
}

var uiConfigPath = regexp.MustCompile("^/uiconfig$")
Expand Down Expand Up @@ -142,7 +154,7 @@ func Handler(cfg Config) http.Handler {
OIDCAutoLogin: oidcConf.AutoLogin,
OIDCLoginEnabled: oidcConf.Enabled,
OIDCButtonText: oidcConf.ButtonText,
FeatureFlags: cfg.Flags,
FeatureFlags: cfg.GetFeatureFlags(),
}
if cfg.NodeID != nil {
args.NodeID = cfg.NodeID.String()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ import {
selectResolution10sStorageTTL,
selectResolution30mStorageTTL,
} from "src/redux/clusterSettings";
import { getDataFromServer } from "src/util/dataFromServer";

interface GraphDashboard {
label: string;
component: (props: GraphDashboardProps) => React.ReactElement<any>[];
Expand Down Expand Up @@ -151,6 +153,7 @@ type NodeGraphsProps = RouteComponentProps &
type NodeGraphsState = {
showLowResolutionAlert: boolean;
showDeletedDataAlert: boolean;
filteredDropdownOptions: { value: string; label: string }[];
};

/**
Expand All @@ -162,9 +165,16 @@ export class NodeGraphs extends React.Component<
> {
constructor(props: NodeGraphsProps) {
super(props);
const crossClusterReplicationEnabled =
getDataFromServer().FeatureFlags.cross_cluster_replication_enabled;
this.state = {
showDeletedDataAlert: false,
showLowResolutionAlert: false,
filteredDropdownOptions: crossClusterReplicationEnabled
? dashboardDropdownOptions // Already in the list, no need to filter
: dashboardDropdownOptions.filter(
option => option.label !== "Cross-Cluster Replication",
),
};
}

Expand Down Expand Up @@ -338,7 +348,7 @@ export class NodeGraphs extends React.Component<
<PageConfigItem>
<Dropdown
title="Dashboard"
options={dashboardDropdownOptions}
options={this.state.filteredDropdownOptions}
selected={dashboard}
onChange={this.dashChange}
className="full-size"
Expand Down

0 comments on commit 5cde7e2

Please sign in to comment.