Skip to content

Commit

Permalink
changefeedccl: ensure rangefeed setting is enabled in tests
Browse files Browse the repository at this point in the history
Previously, many tests which create rangefeeds would not
explicitly set the `kv.rangefeed.enabled` setting to be true.
These tests would still work because, by default, rangefeeds
are enabled via span configs. However, it was observed that
span configs are not immediately applied when range splits
occur. This would cause the testing rangefeed reader to
encounter errors and/or timeout on very rare occasions.
See #109306 (comment) for more info.

This change updates these tests to set the `kv.rangefeed.enabled`
cluster setting to be true, which removes the dependency on span
configs.

Closes: #109306
Epic: None
Release note: None
  • Loading branch information
jayshrivastava committed Aug 25, 2023
1 parent 4c46147 commit 0ec2775
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 2 deletions.
1 change: 1 addition & 0 deletions pkg/ccl/changefeedccl/cdceval/expr_eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ func TestEvaluator(t *testing.T) {
defer s.Stopper().Stop(context.Background())

sqlDB := sqlutils.MakeSQLRunner(db)
sqlDB.Exec(t, "SET CLUSTER SETTING kv.rangefeed.enabled = true")
sqlDB.Exec(t, `CREATE TYPE status AS ENUM ('open', 'closed', 'inactive')`)
sqlDB.Exec(t, `
CREATE TABLE foo (
Expand Down
3 changes: 3 additions & 0 deletions pkg/ccl/changefeedccl/cdcevent/event_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ func TestEventDecoder(t *testing.T) {
defer s.Stopper().Stop(context.Background())

sqlDB := sqlutils.MakeSQLRunner(db)
sqlDB.Exec(t, "SET CLUSTER SETTING kv.rangefeed.enabled = true")
sqlDB.Exec(t, `CREATE TYPE status AS ENUM ('open', 'closed', 'inactive')`)
sqlDB.Exec(t, `
CREATE TABLE foo (
Expand Down Expand Up @@ -419,6 +420,7 @@ func TestEventColumnOrderingWithSchemaChanges(t *testing.T) {
defer s.Stopper().Stop(context.Background())

sqlDB := sqlutils.MakeSQLRunner(db)
sqlDB.Exec(t, "SET CLUSTER SETTING kv.rangefeed.enabled = true")
// Use alter column type to force column reordering.
sqlDB.Exec(t, `SET enable_experimental_alter_column_type_general = true`)

Expand Down Expand Up @@ -761,6 +763,7 @@ func BenchmarkEventDecoder(b *testing.B) {
defer s.Stopper().Stop(context.Background())

sqlDB := sqlutils.MakeSQLRunner(db)
sqlDB.Exec(b, "SET CLUSTER SETTING kv.rangefeed.enabled = true")
sqlDB.Exec(b, `
CREATE TABLE foo (
a INT,
Expand Down
1 change: 1 addition & 0 deletions pkg/ccl/changefeedccl/cdctest/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ go_library(
"//pkg/keys",
"//pkg/kv/kvclient/rangefeed",
"//pkg/kv/kvpb",
"//pkg/kv/kvserver",
"//pkg/roachpb",
"//pkg/sql",
"//pkg/sql/catalog",
Expand Down
8 changes: 8 additions & 0 deletions pkg/ccl/changefeedccl/cdctest/row.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangefeed"
"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/sql"
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
Expand All @@ -37,6 +38,13 @@ func MakeRangeFeedValueReader(
) (func(t testing.TB) *kvpb.RangeFeedValue, func()) {
t.Helper()
execCfg := execCfgI.(sql.ExecutorConfig)

// Rangefeeds might still work even when this setting is false because span
// configs may enable them, but relying on span configs can be prone to
// issues as seen in #109507. Therefore, we assert that the cluster setting
// is set.
require.True(t, kvserver.RangefeedEnabled.Get(&execCfg.Settings.SV))

rows := make(chan *kvpb.RangeFeedValue)
ctx, cleanup := context.WithCancel(context.Background())

Expand Down
1 change: 1 addition & 0 deletions pkg/ccl/changefeedccl/parquet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ func TestParquetRows(t *testing.T) {
maxRowGroupSize := int64(2)

sqlDB := sqlutils.MakeSQLRunner(db)
sqlDB.Exec(t, "SET CLUSTER SETTING kv.rangefeed.enabled = true")

for _, tc := range []struct {
testName string
Expand Down
4 changes: 2 additions & 2 deletions pkg/kv/kvserver/replica.go
Original file line number Diff line number Diff line change
Expand Up @@ -1844,8 +1844,8 @@ func (r *Replica) checkExecutionCanProceedForRangeFeed(
} else if err := r.checkSpanInRangeRLocked(ctx, rSpan); err != nil {
return err
} else if !r.isRangefeedEnabledRLocked() && !RangefeedEnabled.Get(&r.store.cfg.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`))
return errors.Errorf("[r%d] rangefeeds require the kv.rangefeed.enabled setting. See %s",
r.RangeID, docs.URL(`change-data-capture.html#enable-rangefeeds-to-reduce-latency`))
} else if err := r.checkTSAboveGCThresholdRLocked(ts, status, false /* isAdmin */); err != nil {
return err
}
Expand Down

0 comments on commit 0ec2775

Please sign in to comment.