Skip to content

Commit

Permalink
cli/demo,cdc: enable rangefeeds by default
Browse files Browse the repository at this point in the history
Fixes #82719,
which made it annoying to run changefeeds in cockroach demo
as you need to enable rangefeeds twice, once at the system
level. Now cockroach demo enables rangefeeds at startup.

You can override this behavior for performance or to demo
the process of enabling rangefeeds with the flag
`--auto-enable-rangefeeds=false`.

Release note (cli change): cockroach demo now enables rangefeeds by default. You can restore the old behavior with --auto-enable-rangefeeds=false.
  • Loading branch information
HonoreDB committed Jun 23, 2022
1 parent 558370b commit 1635252
Show file tree
Hide file tree
Showing 8 changed files with 79 additions and 0 deletions.
6 changes: 6 additions & 0 deletions pkg/cli/cliflags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -1295,6 +1295,12 @@ and the system tenant using the \connect command.`,
If set, disable enterprise features.`,
}

DemoEnableRangefeeds = FlagInfo{
Name: "auto-enable-rangefeeds",
Description: `
If set to false, overrides the default demo behavior of enabling rangefeeds.`,
}

UseEmptyDatabase = FlagInfo{
Name: "empty",
Description: `Deprecated in favor of --no-example-database`,
Expand Down
1 change: 1 addition & 0 deletions pkg/cli/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,7 @@ func setDemoContextDefaults() {
demoCtx.HTTPPort, _ = strconv.Atoi(base.DefaultHTTPPort)
demoCtx.WorkloadMaxQPS = 25
demoCtx.Multitenant = true
demoCtx.DefaultEnableRangefeeds = true

demoCtx.disableEnterpriseFeatures = false
}
Expand Down
6 changes: 6 additions & 0 deletions pkg/cli/demo.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,12 @@ func runDemo(cmd *cobra.Command, gen workload.Generator) (resErr error) {
}
sqlCtx.ShellCtx.DemoCluster = c

if demoCtx.DefaultEnableRangefeeds {
if err = c.SetClusterSetting(ctx, "kv.rangefeed.enabled", true); err != nil {
return clierrorplus.CheckAndMaybeShout(err)
}
}

if cliCtx.IsInteractive {
cliCtx.PrintfUnlessEmbedded(`#
# Welcome to the CockroachDB demo database!
Expand Down
4 changes: 4 additions & 0 deletions pkg/cli/democluster/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ type DemoCluster interface {

// SetupWorkload initializes the workload generator if defined.
SetupWorkload(ctx context.Context) error

// SetClusterSetting overrides a default cluster setting at system level
// and for all tenants.
SetClusterSetting(ctx context.Context, setting string, value interface{}) error
}

// EnableEnterprise is not implemented here in order to keep OSS/BSL builds successful.
Expand Down
4 changes: 4 additions & 0 deletions pkg/cli/democluster/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ type Context struct {
// Multitenant is true if we're starting the demo cluster in
// multi-tenant mode.
Multitenant bool

// DefaultEnableRangefeeds is true if rangefeeds should start
// out enabled.
DefaultEnableRangefeeds bool
}

// IsInteractive returns true if the demo cluster configuration
Expand Down
20 changes: 20 additions & 0 deletions pkg/cli/democluster/demo_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -1217,6 +1217,26 @@ func (c *transientCluster) maybeEnableMultiTenantMultiRegion(ctx context.Context
return nil
}

func (c *transientCluster) SetClusterSetting(ctx context.Context, setting string, value interface{}) error {
storageURL, err := c.getNetworkURLForServer(ctx, 0, false /* includeAppName */, false /* isTenant */)
if err != nil {
return err
}
db, err := gosql.Open("postgres", storageURL.ToPQ().String())
if err != nil {
return err
}
defer db.Close()
_, err = db.Exec(fmt.Sprintf("SET CLUSTER SETTING %s = '%v'", setting, value))
if err != nil {
return err
}
if c.demoCtx.Multitenant {
_, err = db.Exec(fmt.Sprintf("ALTER TENANT ALL SET CLUSTER SETTING %s = '%v'", setting, value))
}
return err
}

func (c *transientCluster) SetupWorkload(ctx context.Context) error {
if err := c.maybeEnableMultiTenantMultiRegion(ctx); err != nil {
return err
Expand Down
1 change: 1 addition & 0 deletions pkg/cli/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,7 @@ func init() {
"For details, see: "+build.MakeIssueURL(53404))

cliflagcfg.BoolFlag(f, &demoCtx.disableEnterpriseFeatures, cliflags.DemoNoLicense)
cliflagcfg.BoolFlag(f, &demoCtx.DefaultEnableRangefeeds, cliflags.DemoEnableRangefeeds)

cliflagcfg.BoolFlag(f, &demoCtx.Multitenant, cliflags.DemoMultitenant)
// TODO(knz): Currently the multitenant UX for 'demo' is not
Expand Down
37 changes: 37 additions & 0 deletions pkg/cli/interactive_tests/test_demo_changefeeds.tcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#! /usr/bin/env expect -f

source [file join [file dirname $argv0] common.tcl]

start_test "Demo core changefeed using format=csv"
spawn $argv demo --format=csv

# We should start in a populated database.
eexpect "movr>"

# initial_scan=only prevents the changefeed from hanging waiting for more changes.
send "CREATE CHANGEFEED FOR users WITH initial_scan='only';\r"

# header for the results of a successful changefeed
eexpect "table,key,value"

send_eof
eexpect eof

end_test

start_test "Demo with rangefeeds disabled as they are in real life"
spawn $argv demo --format=csv --auto-enable-rangefeeds=false

# We should start in a populated database.
eexpect "movr>"

# initial_scan=only prevents the changefeed from hanging waiting for more changes.
send "CREATE CHANGEFEED FOR users WITH initial_scan='only';\r"

# changefeed should fail fast with an informative error.
eexpect "ERROR: rangefeeds require the kv.rangefeed.enabled setting."

send_eof
eexpect eof

end_test

0 comments on commit 1635252

Please sign in to comment.