Skip to content

Commit

Permalink
Merge #94455
Browse files Browse the repository at this point in the history 
94455: cli/demo: misc fixes r=dhartunian a=knz

Fixes #94308.
Fixes #94835.
Epic: CRDB-22814

See the individual commits for details.

Co-authored-by: Raphael 'kena' Poss <[email protected]>
  • Loading branch information
@knz
craig[bot] and knz committed Jan 6, 2023
2 parents 39ee43d + 3c2bff1 commit 7317b25
Show file tree
Hide file tree
Showing 16 changed files with 484 additions and 138 deletions.
15 changes: 12 additions & 3 deletions pkg/base/test_server_args.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,15 @@ type TestServerArgs struct {
// DisableTLSForHTTP if set, disables TLS for the HTTP interface.
DisableTLSForHTTP bool

// SecondaryTenantPortOffset if non-zero forces the network addresses
// generated for servers started by the serverController to be offset
// from the base addressed by the specified amount.
SecondaryTenantPortOffset int

// SecondaryTenantKnobs contains the testing knobs to use
// for tenant servers started by the serverController.
SecondaryTenantKnobs TestingKnobs

// JoinAddr is the address of a node we are joining.
//
// If left empty and the TestServer is being added to a nonempty cluster, this
Expand Down Expand Up @@ -331,7 +340,7 @@ type TestTenantArgs struct {
// Cockroach Labs. Should remain disabled during unit testing.
StartDiagnosticsReporting bool

// InProcessTenant is used to initialize tenants through the
// kv server's `serverController` in the same process.
InProcessTenant bool
// UseServerController tells testserver.StartTenant() to use
// its serverController to start the secondary tenant.
UseServerController bool
}
12 changes: 5 additions & 7 deletions pkg/cli/cliflags/flags.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1343,16 +1343,14 @@ More information about the geo-partitioned replicas topology can be found at:
Description: `
If set, cockroach demo will start separate in-memory KV and SQL servers in multi-tenancy mode.
The SQL shell will be connected to the first tenant, and can be switched between tenants
and the system tenant using the \connect command. By default the tenant will be initialized
in-process. Use --simulate-separate-tenant-process to emulate a separate tenant process.
`,
and the system tenant using the \connect command.`,
}

DemoInProcessTenant = FlagInfo{
Name: "in-process-tenant",
DemoDisableServerController = FlagInfo{
Name: "disable-server-controller",
Description: `
If set, and --multitenant is set to true, the tenant process will be started
in-process.`,
If set, the server controller will not be used to start secondary
tenant servers.`,
}

DemoNoLicense = FlagInfo{
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/context.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,7 @@ func setDemoContextDefaults() {
demoCtx.HTTPPort, _ = strconv.Atoi(base.DefaultHTTPPort)
demoCtx.WorkloadMaxQPS = 25
demoCtx.Multitenant = true
demoCtx.InProcessTenant = true
demoCtx.DisableServerController = false
demoCtx.DefaultEnableRangefeeds = true

demoCtx.pidFile = ""
Expand Down
3 changes: 3 additions & 0 deletions pkg/cli/demo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,5 +375,8 @@ func runDemoInternal(
defer c.SetSimulatedLatency(false /* on */)
}

// Ensure the last few entries in the log files are flushed at the end.
defer log.Flush()

return sqlCtx.Run(ctx, conn)
}
2 changes: 2 additions & 0 deletions pkg/cli/democluster/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ go_library(
"demo_cluster.go",
"demo_locality_list.go",
"doc.go",
"session_persistence.go",
"socket_unix.go",
"socket_windows.go",
],
Expand Down Expand Up @@ -53,6 +54,7 @@ go_library(
"@com_github_cockroachdb_errors//:errors",
"@com_github_cockroachdb_errors//oserror",
"@com_github_cockroachdb_logtags//:logtags",
"@com_github_cockroachdb_redact//:redact",
"@com_github_nightlyone_lockfile//:lockfile",
"@org_golang_x_time//rate",
],
Expand Down
5 changes: 3 additions & 2 deletions pkg/cli/democluster/context.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,9 @@ type Context struct {
// out enabled.
DefaultEnableRangefeeds bool

// InProcessTenant is true if we want to emulate an in-process tenant.
InProcessTenant bool
// DisableServerController is true if we want to avoid the server
// controller to instantiate tenant secondary servers.
DisableServerController bool
}

// IsInteractive returns true if the demo cluster configuration
Expand Down
104 changes: 75 additions & 29 deletions pkg/cli/democluster/demo_cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,9 @@ func NewDemoCluster(
// tenant.
// Note: this logic can be removed once we use a single
// listener for HTTP and SQL.
c.httpFirstPort += c.demoCtx.NumNodes
if c.demoCtx.DisableServerController {
c.httpFirstPort += c.demoCtx.NumNodes
}
c.sqlFirstPort += c.demoCtx.NumNodes
c.rpcFirstPort += c.demoCtx.NumNodes
}
Expand Down Expand Up @@ -419,23 +421,16 @@ func (c *transientCluster) Start(ctx context.Context) (err error) {
latencyMap := c.servers[i].Cfg.TestingKnobs.Server.(*server.TestingKnobs).
ContextTestingKnobs.InjectedLatencyOracle
c.infoLog(ctx, "starting tenant node %d", i)
tenantStopper := stop.NewStopper()
ts, err := c.servers[i].StartTenant(ctx, base.TestTenantArgs{

args := base.TestTenantArgs{
// We set the tenant ID to i+2, since tenant 0 is not a tenant, and
// tenant 1 is the system tenant. We also subtract 2 for the "starting"
// SQL/HTTP ports so the first tenant ends up with the desired default
// ports.
DisableCreateTenant: !createTenant,
TenantName: roachpb.TenantName(fmt.Sprintf("demo-tenant-%d", secondaryTenantID)),
TenantID: roachpb.MustMakeTenantID(secondaryTenantID),
Stopper: tenantStopper,
ForceInsecure: c.demoCtx.Insecure,
SSLCertsDir: c.demoDir,
DisableTLSForHTTP: true,
EnableDemoLoginEndpoint: true,
StartingRPCAndSQLPort: c.demoCtx.SQLPort - secondaryTenantID + i,
StartingHTTPPort: c.demoCtx.HTTPPort - secondaryTenantID + i,
Locality: c.demoCtx.Localities[i],
DisableCreateTenant: !createTenant,
TenantName: roachpb.TenantName("demo-tenant"),
TenantID: roachpb.MustMakeTenantID(secondaryTenantID),
UseServerController: !c.demoCtx.DisableServerController,
TestingKnobs: base.TestingKnobs{
Server: &server.TestingKnobs{
ContextTestingKnobs: rpc.ContextTestingKnobs{
Expand All @@ -444,15 +439,33 @@ func (c *transientCluster) Start(ctx context.Context) (err error) {
},
},
},
InProcessTenant: c.demoCtx.InProcessTenant,
})
c.stopper.AddCloser(stop.CloserFn(func() {
stopCtx := context.Background()
if ts != nil {
stopCtx = ts.AnnotateCtx(stopCtx)
}
tenantStopper.Stop(stopCtx)
}))
}

var tenantStopper *stop.Stopper
if c.demoCtx.DisableServerController {
tenantStopper = stop.NewStopper()
args.Stopper = tenantStopper
args.ForceInsecure = c.demoCtx.Insecure
args.SSLCertsDir = c.demoDir
args.DisableTLSForHTTP = true
args.EnableDemoLoginEndpoint = true
args.StartingRPCAndSQLPort = c.demoCtx.SQLPort - secondaryTenantID + i
args.StartingHTTPPort = c.demoCtx.HTTPPort - secondaryTenantID + i
args.Locality = c.demoCtx.Localities[i]
}

ts, err := c.servers[i].StartTenant(ctx, args)
if c.demoCtx.DisableServerController {
// If we use the server controller, it is already taking
// care of shutdown.
c.stopper.AddCloser(stop.CloserFn(func() {
stopCtx := context.Background()
if ts != nil {
stopCtx = ts.AnnotateCtx(stopCtx)
}
tenantStopper.Stop(stopCtx)
}))
}
if err != nil {
return err
}
Expand Down Expand Up @@ -525,6 +538,18 @@ func (c *transientCluster) Start(ctx context.Context) (err error) {
}(phaseCtx); err != nil {
return err
}

// Step 10: restore web sessions.
phaseCtx = logtags.AddTag(ctx, "phase", 10)
if err := func(ctx context.Context) error {
if err := c.restoreWebSessions(ctx); err != nil {
c.warnLog(ctx, "unable to restore web sessions: %v", err)
}
return nil
}(phaseCtx); err != nil {
return err
}

return nil
}

Expand Down Expand Up @@ -589,10 +614,14 @@ func (c *transientCluster) createAndAddNode(
// The latency map will be populated after all servers have
// started listening on RPC, and before they proceed with their
// startup routine.
serverKnobs.ContextTestingKnobs = rpc.ContextTestingKnobs{
rpcKnobs := rpc.ContextTestingKnobs{
InjectedLatencyOracle: regionlatency.MakeAddrMap(),
InjectedLatencyEnabled: c.latencyEnabled.Get,
}
serverKnobs.ContextTestingKnobs = rpcKnobs
args.SecondaryTenantKnobs.Server = &server.TestingKnobs{
ContextTestingKnobs: rpcKnobs,
}
}

// Create the server instance. This also registers the in-memory store
Expand Down Expand Up @@ -794,6 +823,7 @@ func (demoCtx *Context) testServerArgsForTransientCluster(
// Demo clusters by default will create their own tenants, so we
// don't need to create them here.
DisableDefaultTestTenant: true,

Knobs: base.TestingKnobs{
Server: &server.TestingKnobs{
StickyEngineRegistry: stickyEngineRegistry,
Expand Down Expand Up @@ -827,6 +857,15 @@ func (demoCtx *Context) testServerArgsForTransientCluster(
args.Addr = fmt.Sprintf("127.0.0.1:%d", rpcPort)
args.SQLAddr = fmt.Sprintf("127.0.0.1:%d", sqlPort)
args.HTTPAddr = fmt.Sprintf("127.0.0.1:%d", httpPort)

if !demoCtx.DisableServerController {
// The code in NewDemoCluster put the KV ports higher
// so we need to subtract the number of nodes to get
// back to the "good" ports.
// We reduce NumNodes by 1 because the server controller
// uses 1-based indexing for servers.
args.SecondaryTenantPortOffset = -(demoCtx.NumNodes + 1)
}
}

if demoCtx.Localities != nil {
Expand All @@ -852,6 +891,9 @@ func TestingForceRandomizeDemoPorts() func() {
}

func (c *transientCluster) Close(ctx context.Context) {
if err := c.saveWebSessions(ctx); err != nil {
c.warnLog(ctx, "unable to save web sessions: %v", err)
}
if c.stopper != nil {
if r := recover(); r != nil {
// A panic here means some of the async tasks may still be
Expand Down Expand Up @@ -1778,6 +1820,11 @@ func (c *transientCluster) ListDemoNodes(w, ew io.Writer, justOne, verbose bool)
// Connection parameters for the system tenant follow.

uiURL := s.Cfg.AdminURL()
if q := uiURL.Query(); c.demoCtx.Multitenant && !c.demoCtx.DisableServerController && !q.Has(server.TenantNameParamInQueryURL) {
q.Add(server.TenantNameParamInQueryURL, catconstants.SystemTenantName)
uiURL.RawQuery = q.Encode()
}

sqlURL, err := c.getNetworkURLForServer(context.Background(), i,
false /* includeAppName */, false /* forSecondaryTenant */)
if err != nil {
Expand Down Expand Up @@ -1809,12 +1856,11 @@ func (c *transientCluster) printURLs(
// Print node ID and web UI URL. Embed the autologin feature inside the URL.
// We avoid printing those when insecure, as the autologin path is not available
// in that case.
pwauth := url.Values{
"username": []string{c.adminUser.Normalized()},
"password": []string{c.adminPassword},
}
q := uiURL.Query()
q.Add("username", c.adminUser.Normalized())
q.Add("password", c.adminPassword)
uiURL.Path = server.DemoLoginPath
uiURL.RawQuery = pwauth.Encode()
uiURL.RawQuery = q.Encode()
}
fmt.Fprintln(w, " (webui) ", uiURL)

Expand Down
47 changes: 25 additions & 22 deletions pkg/cli/democluster/demo_cluster_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,18 @@ func TestTestServerArgsForTransientCluster(t *testing.T) {
sqlPoolMemorySize: 2 << 10,
cacheSize: 1 << 10,
expected: base.TestServerArgs{
DisableDefaultTestTenant: true,
PartOfCluster: true,
JoinAddr: "127.0.0.1",
DisableTLSForHTTP: true,
Addr: "127.0.0.1:7890",
SQLAddr: "127.0.0.1:1234",
HTTPAddr: "127.0.0.1:4567",
SQLMemoryPoolSize: 2 << 10,
CacheSize: 1 << 10,
NoAutoInitializeCluster: true,
EnableDemoLoginEndpoint: true,
DisableDefaultTestTenant: true,
PartOfCluster: true,
JoinAddr: "127.0.0.1",
DisableTLSForHTTP: true,
Addr: "127.0.0.1:7890",
SQLAddr: "127.0.0.1:1234",
HTTPAddr: "127.0.0.1:4567",
SecondaryTenantPortOffset: -2,
SQLMemoryPoolSize: 2 << 10,
CacheSize: 1 << 10,
NoAutoInitializeCluster: true,
EnableDemoLoginEndpoint: true,
Knobs: base.TestingKnobs{
Server: &server.TestingKnobs{
StickyEngineRegistry: stickyEnginesRegistry,
Expand All @@ -91,17 +92,18 @@ func TestTestServerArgsForTransientCluster(t *testing.T) {
sqlPoolMemorySize: 4 << 10,
cacheSize: 4 << 10,
expected: base.TestServerArgs{
DisableDefaultTestTenant: true,
PartOfCluster: true,
JoinAddr: "127.0.0.1",
Addr: "127.0.0.1:7892",
SQLAddr: "127.0.0.1:1236",
HTTPAddr: "127.0.0.1:4569",
DisableTLSForHTTP: true,
SQLMemoryPoolSize: 4 << 10,
CacheSize: 4 << 10,
NoAutoInitializeCluster: true,
EnableDemoLoginEndpoint: true,
DisableDefaultTestTenant: true,
PartOfCluster: true,
JoinAddr: "127.0.0.1",
Addr: "127.0.0.1:7892",
SQLAddr: "127.0.0.1:1236",
HTTPAddr: "127.0.0.1:4569",
SecondaryTenantPortOffset: -2,
DisableTLSForHTTP: true,
SQLMemoryPoolSize: 4 << 10,
CacheSize: 4 << 10,
NoAutoInitializeCluster: true,
EnableDemoLoginEndpoint: true,
Knobs: base.TestingKnobs{
Server: &server.TestingKnobs{
StickyEngineRegistry: stickyEnginesRegistry,
Expand Down Expand Up @@ -257,6 +259,7 @@ func TestTransientClusterMultitenant(t *testing.T) {

// This test is too slow to complete under the race detector, sometimes.
skip.UnderRace(t)
skip.UnderStress(t)

demoCtx := newDemoCtx()
// Set up an empty 3-node cluster with tenants on each node.
Expand Down
Loading

0 comments on commit 7317b25

Please sign in to comment.