Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: make --global flag for demo public #62435

Merged
merged 4 commits into from
Apr 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions pkg/cli/demo.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,12 @@ func checkDemoConfiguration(

// Make sure the number of nodes is valid.
if demoCtx.nodes <= 0 {
return nil, errors.Newf("--nodes has invalid value (expected positive, got %d)", demoCtx.nodes)
return nil, errors.Newf("--%s has invalid value (expected positive, got %d)", cliflags.DemoNodes.Name, demoCtx.nodes)
}

// If artificial latencies were requested, then the user cannot supply their own localities.
if demoCtx.simulateLatency && demoCtx.localities != nil {
return nil, errors.New("--global cannot be used with --demo-locality")
return nil, errors.Newf("--%s cannot be used with --%s", cliflags.Global.Name, cliflags.DemoNodeLocality.Name)
}

demoCtx.disableTelemetry = cluster.TelemetryOptOut()
Expand Down Expand Up @@ -301,7 +301,10 @@ func runDemo(cmd *cobra.Command, gen workload.Generator) (err error) {

if demoCtx.simulateLatency {
printfUnlessEmbedded(
"#\n# WARNING: the use of --%s is experimental. Some features may not work as expected.\n",
`# Communication between nodes will simulate real world latencies.
#
# WARNING: the use of --%s is experimental. Some features may not work as expected.
`,
cliflags.Global.Name,
)
}
Expand Down
107 changes: 107 additions & 0 deletions pkg/cli/demo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,20 @@ package cli
import (
"context"
"fmt"
"io/ioutil"
"testing"
"time"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/server"
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/stop"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestTestServerArgsForTransientCluster(t *testing.T) {
Expand Down Expand Up @@ -113,3 +119,104 @@ func TestTestServerArgsForTransientCluster(t *testing.T) {
})
}
}

func TestTransientClusterSimulateLatencies(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

// This is slow under race as it starts a 9-node cluster which
// has a very high simulated latency between each node.
skip.UnderRace(t)

// Ensure flags are reset on start, and also make sure
// at the end of the test they are reset.
TestingReset()
defer TestingReset()

// Set up an empty 9-node cluster with simulated latencies.
demoCtx.simulateLatency = true
demoCtx.nodes = 9

certsDir, err := ioutil.TempDir("", "cli-demo-test")
require.NoError(t, err)

cleanupFunc := createTestCerts(certsDir)
defer func() {
if err := cleanupFunc(); err != nil {
t.Fatal(err)
}
}()

ctx := context.Background()

// Setup the transient cluster.
c := transientCluster{
stopper: stop.NewStopper(),
demoDir: certsDir,
stickyEngineRegistry: server.NewStickyInMemEnginesRegistry(),
}
// Stop the cluster when the test exits, including when it fails.
// This also calls the Stop() method on the stopper, and thus
// cancels everything controlled by the stopper.
defer c.cleanup(ctx)

// Also ensure the context gets canceled when the stopper
// terminates above.
ctx, _ = c.stopper.WithCancelOnQuiesce(ctx)

require.NoError(t, c.start(ctx, demoCmd, nil /* gen */))

for _, tc := range []struct {
desc string
nodeIdx int
region string
}{
{
desc: "from us-east1",
nodeIdx: 0,
region: "us-east1",
},
{
desc: "from us-west1",
nodeIdx: 3,
region: "us-west1",
},
{
desc: "from europe-west1",
nodeIdx: 6,
region: "europe-west1",
},
} {
t.Run(tc.desc, func(t *testing.T) {
url, err := c.getNetworkURLForServer(tc.nodeIdx, nil /* gen */, true /* includeAppName */)
require.NoError(t, err)
conn := makeSQLConn(url)
defer conn.Close()
// Find the maximum latency in the cluster from the current node.
var maxLatency time.Duration
for _, latencyMS := range regionToRegionToLatency[tc.region] {
if d := time.Duration(latencyMS) * time.Millisecond; d > maxLatency {
maxLatency = d
}
}

// Attempt to make a query that talks to every node.
// This should take at least maxLatency.
startTime := timeutil.Now()
_, _, err = runQuery(
conn,
makeQuery(`SHOW ALL CLUSTER QUERIES`),
false,
)
totalDuration := timeutil.Since(startTime)
require.NoError(t, err)
require.Truef(
t,
totalDuration >= maxLatency,
"expected duration at least %s, got %s",
maxLatency,
totalDuration,
)
})
}
}
2 changes: 0 additions & 2 deletions pkg/cli/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -766,9 +766,7 @@ func init() {
"For details, see: "+build.MakeIssueURL(53404))

boolFlag(f, &demoCtx.disableLicenseAcquisition, cliflags.DemoNoLicense)
// Mark the --global flag as hidden until we investigate it more.
boolFlag(f, &demoCtx.simulateLatency, cliflags.Global)
_ = f.MarkHidden(cliflags.Global.Name)
// The --empty flag is only valid for the top level demo command,
// so we use the regular flag set.
boolFlag(demoCmd.Flags(), &demoCtx.noExampleDatabase, cliflags.UseEmptyDatabase)
Expand Down