-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
Browse the repository at this point in the history
84888: distsql: remove queueing in the flow scheduler r=yuzefovich a=yuzefovich **flowinfra: de-duplicate cluster flow tests** Previously, there was a lot of duplication for two cluster flow tests (one for the single tenant setup and another for the multi tenant setup), and this commit cleans it up. Release note: None **distsql: remove queueing in the flow scheduler** This commit removes the queueing behavior in the flow scheduler (which is now renamed to "remote flow runner" to better reflect its purpose). This behavior was already disabled on 22.2 (with a cluster setting that could enable it back as an escape hatch) since we wanted to be conservative when removing it, but I don't foresee any problems with this, so it should be safe to remove it. We don't remove the "cancel dead flow coordinator" since it might still be useful in a mixed-version cluster, but more importantly the coordinator will be refactored to also cancel the running flows (not just the queued flows as it it used to). (This change will be in a separate commit.) This required removal some of the tests that relied on the queueing behavior, but I don't think we're losing much test coverage. Fixes: #34229. Release note (sql change): `sql.distsql.max_running_flows` cluster setting has been removed. This setting previously controlled the number of remote DistSQL flows that a single node would run at any time. Once that number was exceeded, the incoming flows would get queued until the number was reduced. This was used as a poor man's version of the admission control, but now that we have an actual admission control in place, we don't need that queueing behavior. 89324: roachtest: add admissioncontrol/index-overload r=andrewbaptist a=andrewbaptist This test sets up a 3 node cluster and measures the impact of creating an index while a controlled KV workload is running. The test measures two things * The baseline KV workload P99 latency * The impact on running index creation on the workload. The KV workload is designed to use about 20% of the CPU and IO resources of the system. Index creation is impactful by both reading a lot of data and writing a large index, however the primary impact is that it causes enough L0 inversion to make user traffic pause. Release note: None 89630: eval: clean up usage of UnwrapDatum r=yuzefovich a=yuzefovich This commit removes many calls to `eval.UnwrapDatum` where the first argument was `nil`. In such a case that function is equivalent to `tree.UnwrapDOidWrapper`, so this commit uses the latter wherever possible. Next, this commit adds an explicit `context.Context` argument to the signature to avoid the usage of the deprecated stored context from `eval.Context`. This required a little bit of plumbing around the index encoding methods. Release note: None Epic: None 89648: server: allow configuring vmodule via cluster setting r=dt a=dt Closes #89298. Release note: none. Epic: none. 90659: backfill: retain dropping columns when validating inverted index r=Xiang-Gu a=Xiang-Gu Previously, when validating a forward index, we made the first mutation(s) public with two filters: ignore constraints and retain dropping columns. But we forgot to include the retain-dropping-columns policy when validating *inverted* index. This will only manifest itself in rare cases involving dropping column and validating inverted indexes. An example to trigger this rare case is: ``` create table t (j int[], k int not null, inverted index (j)); alter table t alter primary key using columns (k); ``` Fixes #90306 Release note: None Co-authored-by: Yahor Yuzefovich <[email protected]> Co-authored-by: Andrew Baptist <[email protected]> Co-authored-by: David Taylor <[email protected]> Co-authored-by: Xiang Gu <[email protected]>
- Loading branch information
Showing
61 changed files
with
741 additions
and
1,740 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
118 changes: 118 additions & 0 deletions
118
pkg/cmd/roachtest/tests/admission_control_index_overload.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| // Copyright 2022 The Cockroach Authors. | ||
| // | ||
| // Use of this software is governed by the Business Source License | ||
| // included in the file licenses/BSL.txt. | ||
| // | ||
| // As of the Change Date specified in that file, in accordance with | ||
| // the Business Source License, use of this software will be governed | ||
| // by the Apache License, Version 2.0, included in the file | ||
| // licenses/APL.txt. | ||
|
|
||
| package tests | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "time" | ||
|
|
||
| "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster" | ||
| "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option" | ||
| "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry" | ||
| "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/spec" | ||
| "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test" | ||
| "github.com/cockroachdb/cockroach/pkg/roachprod/install" | ||
| "github.com/cockroachdb/cockroach/pkg/roachprod/prometheus" | ||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| // This test sets up a 3-node CRDB cluster on 8vCPU machines, loads it up with a | ||
| // large TPC-C dataset, and sets up a foreground load of kv50/1b. It then | ||
| // attempts to create a useless secondary index on the table while the workload | ||
| // is running to measure the impact. The index will not be used by any of the | ||
| // queries, but the intent is to measure the impact of the index creation. | ||
| func registerIndexOverload(r registry.Registry) { | ||
| r.Add(registry.TestSpec{ | ||
| Name: "admission-control/index-overload", | ||
| Owner: registry.OwnerAdmissionControl, | ||
| // TODO(baptist): After two weeks of nightly baking time, reduce | ||
| // this to a weekly cadence. This is a long-running test and serves only | ||
| // as a coarse-grained benchmark. | ||
| // Tags: []string{`weekly`}, | ||
| Cluster: r.MakeClusterSpec(4, spec.CPU(8)), | ||
| Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { | ||
| crdbNodes := c.Spec().NodeCount - 1 | ||
| workloadNode := c.Spec().NodeCount | ||
|
|
||
| c.Put(ctx, t.Cockroach(), "./cockroach", c.All()) | ||
| c.Start(ctx, t.L(), option.DefaultStartOpts(), install.MakeClusterSettings(), c.Range(1, crdbNodes)) | ||
|
|
||
| { | ||
| promCfg := &prometheus.Config{} | ||
| promCfg.WithPrometheusNode(c.Node(workloadNode).InstallNodes()[0]) | ||
| promCfg.WithNodeExporter(c.All().InstallNodes()) | ||
| promCfg.WithCluster(c.Range(1, crdbNodes).InstallNodes()) | ||
| promCfg.WithGrafanaDashboard("http://go.crdb.dev/p/snapshot-admission-control-grafana") | ||
| promCfg.ScrapeConfigs = append(promCfg.ScrapeConfigs, prometheus.MakeWorkloadScrapeConfig("workload", | ||
| "/", makeWorkloadScrapeNodes(c.Node(workloadNode).InstallNodes()[0], []workloadInstance{ | ||
| {nodes: c.Node(workloadNode)}, | ||
| }))) | ||
| _, cleanupFunc := setupPrometheusForRoachtest(ctx, t, c, promCfg, []workloadInstance{{nodes: c.Node(workloadNode)}}) | ||
| defer cleanupFunc() | ||
| } | ||
|
|
||
| duration, err := time.ParseDuration(ifLocal(c, "20s", "10m")) | ||
| assert.NoError(t, err) | ||
| testDuration := 3 * duration | ||
|
|
||
| db := c.Conn(ctx, t.L(), crdbNodes) | ||
| defer db.Close() | ||
|
|
||
| if !t.SkipInit() { | ||
| t.Status("initializing kv dataset ", time.Minute) | ||
| splits := ifLocal(c, " --splits=3", " --splits=100") | ||
| c.Run(ctx, c.Node(workloadNode), "./cockroach workload init kv "+splits+" {pgurl:1}") | ||
|
|
||
| // We need a big enough size so index creation will take enough time. | ||
| t.Status("initializing tpcc dataset ", duration) | ||
| warehouses := ifLocal(c, " --warehouses=1", " --warehouses=2000") | ||
| c.Run(ctx, c.Node(workloadNode), "./cockroach workload fixtures import tpcc --checks=false"+warehouses+" {pgurl:1}") | ||
|
|
||
| // Setting this low allows us to hit overload. In a larger cluster with | ||
| // more nodes and larger tables, it will hit the unmodified 1000 limit. | ||
| // TODO(baptist): Ideally lower the default setting to 10. Once that is | ||
| // done, then this block can be removed. | ||
| if _, err := db.ExecContext(ctx, | ||
| "SET CLUSTER SETTING admission.l0_file_count_overload_threshold=10", | ||
| ); err != nil { | ||
| t.Fatalf("failed to alter cluster setting: %v", err) | ||
| } | ||
| } | ||
|
|
||
| t.Status("starting kv workload thread to run for ", testDuration) | ||
| m := c.NewMonitor(ctx, c.Range(1, crdbNodes)) | ||
| m.Go(func(ctx context.Context) error { | ||
| testDurationStr := " --duration=" + testDuration.String() | ||
| concurrency := ifLocal(c, " --concurrency=8", " --concurrency=2048") | ||
| c.Run(ctx, c.Node(crdbNodes+1), | ||
| "./cockroach workload run kv --read-percent=50 --max-rate=1000 --max-block-bytes=4096"+ | ||
| testDurationStr+concurrency+fmt.Sprintf(" {pgurl:1-%d}", crdbNodes), | ||
| ) | ||
| return nil | ||
| }) | ||
|
|
||
| t.Status("recording baseline performance ", duration) | ||
| time.Sleep(duration) | ||
|
|
||
| // Choose an index creation that takes ~10-12 minutes. | ||
| t.Status("starting index creation ", duration) | ||
| if _, err := db.ExecContext(ctx, | ||
| "CREATE INDEX test_index ON tpcc.stock(s_quantity)", | ||
| ); err != nil { | ||
| t.Fatalf("failed to create index: %v", err) | ||
| } | ||
|
|
||
| t.Status("index creation complete - waiting for workload to finish ", duration) | ||
| m.Wait() | ||
| }, | ||
| }) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.