Skip to content

Commit

Permalink
cli,democluster: defer simulated latency until after cluster setup
Browse files Browse the repository at this point in the history
Cluster creation and tenant setup is chatty. That's an okay thing: we don't
really care about cluster creation being that slow in general. In the case of
demo when we want to simulate latency and use tenants, it was particularly
painful. Starting the 9 tenants would take many minutes. This patch alleviates
this problem by keeping latency between the simulated nodes low until just
before we pass control to the user.

Fixes #76305

Release note (cli change): `cockroach demo --global` will now start up more
quickly. The latency which will be injected is not injected until after the
initial cluster is set up internally.
  • Loading branch information
ajwerner committed Nov 22, 2022
1 parent 2d79db8 commit 137b27b
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 8 deletions.
7 changes: 7 additions & 0 deletions pkg/cli/demo.go
Original file line number Diff line number Diff line change
Expand Up @@ -360,5 +360,12 @@ func runDemoInternal(
return err
}

// Enable the latency as late in the process of starting the cluster as we
// can to minimize the startup time.
if demoCtx.SimulateLatency {
c.SetSimulatedLatency(true /* on */)
defer c.SetSimulatedLatency(false /* on */)
}

return sqlCtx.Run(ctx, conn)
}
1 change: 1 addition & 0 deletions pkg/cli/democluster/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ go_library(
"//pkg/util/netutil/addr",
"//pkg/util/retry",
"//pkg/util/stop",
"//pkg/util/syncutil",
"//pkg/workload",
"//pkg/workload/histogram",
"//pkg/workload/workloadsql",
Expand Down
3 changes: 3 additions & 0 deletions pkg/cli/democluster/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ type DemoCluster interface {
// SetClusterSetting overrides a default cluster setting at system level
// and for all tenants.
SetClusterSetting(ctx context.Context, setting string, value interface{}) error

// SetSimulatedLatency is used to enable or disable simulated latency.
SetSimulatedLatency(on bool)
}

// EnableEnterprise is not implemented here in order to keep OSS/BSL builds successful.
Expand Down
37 changes: 29 additions & 8 deletions pkg/cli/democluster/demo_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,15 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/distsql"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils/regionlatency"
"github.com/cockroachdb/cockroach/pkg/util/humanizeutil"
"github.com/cockroachdb/cockroach/pkg/util/log/logcrash"
"github.com/cockroachdb/cockroach/pkg/util/log/logpb"
"github.com/cockroachdb/cockroach/pkg/util/log/severity"
"github.com/cockroachdb/cockroach/pkg/util/netutil/addr"
"github.com/cockroachdb/cockroach/pkg/util/retry"
"github.com/cockroachdb/cockroach/pkg/util/stop"
"github.com/cockroachdb/cockroach/pkg/util/syncutil"
"github.com/cockroachdb/cockroach/pkg/workload"
"github.com/cockroachdb/cockroach/pkg/workload/histogram"
"github.com/cockroachdb/cockroach/pkg/workload/workloadsql"
Expand Down Expand Up @@ -83,6 +85,10 @@ type transientCluster struct {
infoLog LoggerFn
warnLog LoggerFn
shoutLog ShoutLoggerFn

// latencyEnabled controls whether simulated latency is currently enabled.
// It is only relevant when using SimulateLatency.
latencyEnabled syncutil.AtomicBool
}

// maxNodeInitTime is the maximum amount of time to wait for nodes to
Expand Down Expand Up @@ -315,13 +321,13 @@ func (c *transientCluster) Start(ctx context.Context) (err error) {
phaseCtx = logtags.AddTag(ctx, "phase", 5)
if err := func(ctx context.Context) error {
// If latency simulation is requested, initialize the latency map.
if c.demoCtx.SimulateLatency {
// Now, all servers have been started enough to know their own RPC serving
// addresses, but nothing else. Assemble the artificial latency map.
c.infoLog(ctx, "initializing latency map")
localityLatencies.Apply(c)
if !c.demoCtx.SimulateLatency {
return nil
}
return nil
// Now, all servers have been started enough to know their own RPC serving
// addresses, but nothing else. Assemble the artificial latency map.
c.infoLog(ctx, "initializing latency map")
return localityLatencies.Apply(c)
}(phaseCtx); err != nil {
return err
}
Expand Down Expand Up @@ -389,7 +395,8 @@ func (c *transientCluster) Start(ctx context.Context) (err error) {
TestingKnobs: base.TestingKnobs{
Server: &server.TestingKnobs{
ContextTestingKnobs: rpc.ContextTestingKnobs{
InjectedLatencyOracle: latencyMap,
InjectedLatencyOracle: latencyMap,
InjectedLatencyEnabled: c.latencyEnabled.Get,
},
},
},
Expand Down Expand Up @@ -484,6 +491,19 @@ func (c *transientCluster) Start(ctx context.Context) (err error) {
return nil
}

// SetSimulatedLatency enables or disable the simulated latency and then
// clears the remote clock tracking. If the remote clocks were not cleared,
// bad routing decisions would be made as soon as latency is turned on.
func (c *transientCluster) SetSimulatedLatency(on bool) {
c.latencyEnabled.Set(on)
for _, s := range c.servers {
s.RPCContext().RemoteClocks.TestingResetLatencyInfos()
}
for _, s := range c.tenantServers {
s.RPCContext().RemoteClocks.TestingResetLatencyInfos()
}
}

// createAndAddNode is responsible for determining node parameters,
// instantiating the server component and connecting it to the
// cluster's stopper.
Expand Down Expand Up @@ -531,7 +551,8 @@ func (c *transientCluster) createAndAddNode(
// started listening on RPC, and before they proceed with their
// startup routine.
serverKnobs.ContextTestingKnobs = rpc.ContextTestingKnobs{
InjectedLatencyOracle: make(rpc.InjectedLatencyMap),
InjectedLatencyOracle: regionlatency.MakeAddrMap(),
InjectedLatencyEnabled: c.latencyEnabled.Get,
}
}

Expand Down

0 comments on commit 137b27b

Please sign in to comment.