-
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.
93952: sql: measure CPU time spent during SQL execution r=DrewKimball a=DrewKimball This commit adds tracking for CPU time spent during SQL execution. The CPU time is tracked at the operator granularity when statistics collection is enabled, similar to execution time. For now, the CPU time is only surfaced in the output of `EXPLAIN ANALYZE` variants. A future PR will add support for logging this value in the statement statistics. Informs: #87213 Release note (sql change): CPU time spent during SQL execution is now visible in the output of queries run with `EXPLAIN ANALYZE`. This measure does not include CPU time spent while serving KV requests. This can be useful for diagnosing performance issues and optimizing SQL queries. 95040: tenantcapabilities: introduce a Watcher over system.tenants r=knz a=arulajmani This patch introduces three new interfaces -- a Watcher, a Reader, and Authorizer. They're not hooked up yet, but once they are, they'll work together to provide (in-memory) capability checks for incoming tenant requests. The Watcher establishes a rangefeed over `system.tenants` to incrementally (and transparently) maintain an in-memory view of the global tenant capability state. Publicly, it exposes a `Reader` interface. The `Reader` provides access to the global tenant capability state. The `Watcher` and `Authorizer` communicate with each other using the `Reader` interface. The `Authorizer` consulsts the global tenant capability state to perform authorization checks for incoming requests issued by tenants. Part of the motivation to structure the code as such is to expand the set of inputs the `Authorizer` uses to authorize requests. One could imagine other dependencies being injected into the `Authorizer` in the future. Epic: CRDB-18503 References: #94643 Release note: None 95361: sql/schemachanger: add compatibility with 22.2 rules r=fqazi a=fqazi This PR will make the following changes: 1. Refactor the existing rules package so that there is a common package, and multiple dep/op rules registries to allow us to support rules from older releases. This also includes splitting out helper functions to make this process easier 2. Import the 22.2 rules and have them adopt the same refactoring, so that a new rules/deps registry exists with them 3. Select the dep/op rules based on the active version of CRDB Co-authored-by: Drew Kimball <[email protected]> Co-authored-by: Arul Ajmani <[email protected]> Co-authored-by: Faizan Qazi <[email protected]>
- Loading branch information
Showing
98 changed files
with
10,330 additions
and
3,703 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
33 changes: 33 additions & 0 deletions
33
pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/BUILD.bazel
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,33 @@ | ||
| load("//build/bazelutil/unused_checker:unused.bzl", "get_x_data") | ||
| load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") | ||
|
|
||
| go_library( | ||
| name = "tenantcapabilitiesauthorizer", | ||
| srcs = ["authorizer.go"], | ||
| importpath = "github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer", | ||
| visibility = ["//visibility:public"], | ||
| deps = [ | ||
| "//pkg/multitenant/tenantcapabilities", | ||
| "//pkg/roachpb", | ||
| "//pkg/util/log", | ||
| ], | ||
| ) | ||
|
|
||
| go_test( | ||
| name = "tenantcapabilitiesauthorizer_test", | ||
| srcs = ["authorizer_test.go"], | ||
| args = ["-test.timeout=295s"], | ||
| data = glob(["testdata/**"]), | ||
| embed = [":tenantcapabilitiesauthorizer"], | ||
| deps = [ | ||
| "//pkg/multitenant/tenantcapabilities", | ||
| "//pkg/multitenant/tenantcapabilities/tenantcapabilitiespb", | ||
| "//pkg/multitenant/tenantcapabilities/tenantcapabilitiestestutils", | ||
| "//pkg/roachpb", | ||
| "//pkg/testutils/datapathutils", | ||
| "//pkg/util/leaktest", | ||
| "@com_github_cockroachdb_datadriven//:datadriven", | ||
| ], | ||
| ) | ||
|
|
||
| get_x_data(name = "get_x_data") |
64 changes: 64 additions & 0 deletions
64
pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/authorizer.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,64 @@ | ||
| // Copyright 2023 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 tenantcapabilitiesauthorizer | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| "github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilities" | ||
| "github.com/cockroachdb/cockroach/pkg/roachpb" | ||
| "github.com/cockroachdb/cockroach/pkg/util/log" | ||
| ) | ||
|
|
||
| // Authorizer is a concrete implementation of the tenantcapabilities.Authorizer | ||
| // interface. It's safe for concurrent use. | ||
| type Authorizer struct { | ||
| capabilitiesReader tenantcapabilities.Reader | ||
| } | ||
|
|
||
| var _ tenantcapabilities.Authorizer = &Authorizer{} | ||
|
|
||
| // New constructs a new tenantcapabilities.Authorizer. | ||
| func New(reader tenantcapabilities.Reader) *Authorizer { | ||
| a := &Authorizer{ | ||
| capabilitiesReader: reader, | ||
| } | ||
| return a | ||
| } | ||
|
|
||
| // HasCapabilityForBatch implements the tenantcapabilities.Authorizer interface. | ||
| func (a *Authorizer) HasCapabilityForBatch( | ||
| ctx context.Context, tenID roachpb.TenantID, ba *roachpb.BatchRequest, | ||
| ) bool { | ||
| if tenID.IsSystem() { | ||
| return true // the system tenant is allowed to do as it pleases | ||
| } | ||
| cp, found := a.capabilitiesReader.GetCapabilities(tenID) | ||
| if !found { | ||
| log.Infof( | ||
| ctx, | ||
| "no capability information for tenant %s; requests that require capabilities may be denied", | ||
| tenID, | ||
| ) | ||
| } | ||
|
|
||
| for _, ru := range ba.Requests { | ||
| switch ru.GetInner().(type) { | ||
| case *roachpb.AdminSplitRequest: | ||
| if !cp.CanAdminSplit { | ||
| return false | ||
| } | ||
| default: | ||
| // No capability checks for other types of requests. | ||
| } | ||
| } | ||
| return true | ||
| } |
89 changes: 89 additions & 0 deletions
89
pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer/authorizer_test.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,89 @@ | ||
| // Copyright 2023 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 tenantcapabilitiesauthorizer | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "testing" | ||
|
|
||
| "github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilities" | ||
| "github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilities/tenantcapabilitiespb" | ||
| "github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilities/tenantcapabilitiestestutils" | ||
| "github.com/cockroachdb/cockroach/pkg/roachpb" | ||
| "github.com/cockroachdb/cockroach/pkg/testutils/datapathutils" | ||
| "github.com/cockroachdb/cockroach/pkg/util/leaktest" | ||
| "github.com/cockroachdb/datadriven" | ||
| ) | ||
|
|
||
| // TestDataDriven runs datadriven tests against the Authorizer interface. The | ||
| // syntax is as follows: | ||
| // | ||
| // "update-state": updates the underlying global tenant capability state. | ||
| // Example: | ||
| // | ||
| // update-state | ||
| // upsert {ten=10}:{CanAdminSplit=true} | ||
| // delete {ten=15} | ||
| // ---- | ||
| // | ||
| // "has-capability-for-batch": performs a capability check, given a tenant and | ||
| // batch request declaration. Example: | ||
| // | ||
| // has-capability-for-batch | ||
| // {ten=10} | ||
| // split | ||
| // ---- | ||
| func TestDataDriven(t *testing.T) { | ||
| defer leaktest.AfterTest(t)() | ||
|
|
||
| datadriven.Walk(t, datapathutils.TestDataPath(t), func(t *testing.T, path string) { | ||
| mockReader := mockReader(make(map[roachpb.TenantID]tenantcapabilitiespb.TenantCapabilities)) | ||
| authorizer := New(mockReader) | ||
|
|
||
| datadriven.RunTest(t, path, func(t *testing.T, d *datadriven.TestData) string { | ||
| switch d.Cmd { | ||
| case "update-state": | ||
| updates := tenantcapabilitiestestutils.ParseTenantCapabilityUpdateStateArguments(t, d.Input) | ||
| mockReader.updateState(updates) | ||
|
|
||
| case "has-capability-for-batch": | ||
| tenID, ba := tenantcapabilitiestestutils.ParseBatchRequestString(t, d.Input) | ||
| hasCapability := authorizer.HasCapabilityForBatch(context.Background(), tenID, &ba) | ||
| return fmt.Sprintf("%t", hasCapability) | ||
|
|
||
| default: | ||
| return fmt.Sprintf("unknown command %s", d.Cmd) | ||
| } | ||
| return "ok" | ||
| }) | ||
| }) | ||
| } | ||
|
|
||
| type mockReader map[roachpb.TenantID]tenantcapabilitiespb.TenantCapabilities | ||
|
|
||
| func (m mockReader) updateState(updates []tenantcapabilities.Update) { | ||
| for _, update := range updates { | ||
| if update.Deleted { | ||
| delete(m, update.TenantID) | ||
| } else { | ||
| m[update.TenantID] = update.TenantCapabilities | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // GetCapabilities implements the tenantcapabilities.Reader interface. | ||
| func (m mockReader) GetCapabilities( | ||
| id roachpb.TenantID, | ||
| ) (tenantcapabilitiespb.TenantCapabilities, bool) { | ||
| cp, found := m[id] | ||
| return cp, found | ||
| } |
Oops, something went wrong.