Skip to content

Commit

Permalink
changefeedccl: check that rangefeeds are enabled earlier
Browse files Browse the repository at this point in the history
Pushing this check earlier means that for core changefeeds, the check
happens before the backfill. Previously an error would only be seen
after the backfill when we attempted to start the rangefeed, which
some users found confusing.

Release note: None
  • Loading branch information
stevendanna committed Sep 21, 2021
1 parent 2c36f59 commit e372d9e
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 14 deletions.
30 changes: 16 additions & 14 deletions pkg/ccl/changefeedccl/changefeed_stmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,6 @@ func changefeedPlanHook(
return nil, nil, nil, false, nil
}

if err := featureflag.CheckEnabled(
ctx,
p.ExecCfg(),
featureChangefeedEnabled,
"CHANGEFEED",
); err != nil {
return nil, nil, nil, false, err
}
var sinkURIFn func() (string, error)
var header colinfo.ResultColumns
unspecifiedSink := changefeedStmt.SinkURI == nil
Expand Down Expand Up @@ -132,6 +124,22 @@ func changefeedPlanHook(
ctx, span := tracing.ChildSpan(ctx, stmt.StatementTag())
defer span.Finish()

if err := featureflag.CheckEnabled(
ctx,
p.ExecCfg(),
featureChangefeedEnabled,
"CHANGEFEED",
); err != nil {
return err
}

// Changefeeds are based on the Rangefeed abstraction, which
// requires the `kv.rangefeed.enabled` setting to be true.
if !kvserver.RangefeedEnabled.Get(&p.ExecCfg().Settings.SV) {
return errors.Errorf("rangefeeds require the kv.rangefeed.enabled setting. See %s",
docs.URL(`change-data-capture.html#enable-rangefeeds-to-reduce-latency`))
}

ok, err := p.HasRoleOption(ctx, roleoption.CONTROLCHANGEFEED)
if err != nil {
return err
Expand Down Expand Up @@ -335,12 +343,6 @@ func changefeedPlanHook(
return changefeedbase.MaybeStripRetryableErrorMarker(err)
}

// Changefeeds are based on the Rangefeed abstraction, which requires the
// `kv.rangefeed.enabled` setting to be true.
if !kvserver.RangefeedEnabled.Get(&p.ExecCfg().Settings.SV) {
return errors.Errorf("rangefeeds require the kv.rangefeed.enabled setting. See %s",
docs.URL(`change-data-capture.html#enable-rangefeeds-to-reduce-latency`))
}
if err := utilccl.CheckEnterpriseEnabled(
p.ExecCfg().Settings, p.ExecCfg().ClusterID(), p.ExecCfg().Organization(), "CHANGEFEED",
); err != nil {
Expand Down
1 change: 1 addition & 0 deletions pkg/ccl/changefeedccl/changefeed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,7 @@ func TestChangefeedExternalIODisabled(t *testing.T) {
})
defer s.Stopper().Stop(ctx)
sqlDB := sqlutils.MakeSQLRunner(db)
sqlDB.Exec(t, serverSetupStatements)
sqlDB.Exec(t, "CREATE TABLE target_table (pk INT PRIMARY KEY)")
for _, proto := range disallowedSinkProtos {
sqlDB.ExpectErr(t, "Outbound IO is disabled by configuration, cannot create changefeed",
Expand Down
3 changes: 3 additions & 0 deletions pkg/ccl/logictestccl/testdata/logic_test/changefeed
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
statement ok
CREATE TABLE t ()

statement ok
SET CLUSTER SETTING kv.rangefeed.enabled = true

user testuser

statement error permission denied to create changefeed
Expand Down

0 comments on commit e372d9e

Please sign in to comment.