Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
118603: *: retire multi tenant VTeam attributes r=yuzefovich a=yuzefovich

**: remove all mentions of T-multitenant**

Release note: None

**roachtest: reassign ownership from MultiTenant to DisasterRecovery**

This commit reassigns ownership of the following roachtests:
- `acceptance/multitenant/`
- `multitenant/shared-process/basic`
- `multitenant-upgrade`

to Disaster Recovery team (which is temporary until the Shared Services team if staffed).

Release note: None

**CODEOWNERS: remove cockroachdb/multi-tenant**

This commit removes all mentions of cockroachdb/multi-tenant in favor of cockroachdb/server-prs.

Release note: None

Epic: None

119602: sql: reduce indexed variable allocations r=mgartner a=mgartner

#### execinfrapb: remove unnecessary DefaultIntSize parse option

When `execinfrapb.Expression`s are serialized, references to integer
types are always formatted unambiguously with the
`tree.FmtCheckEquivalence` flag as `INT2`, `INT4`, or `INT8`, and never
as `INT`. Therefore, there is never a need to disambiguate the meaning
of `INT` based on the `default_int_size` session setting. This allows
simplification of the deserialization logic.

Release note: None

#### execinfrapb: remove unnecessary check for aggregates

This commit removes some logic in `execinfrapb` that asserted that
expressions being deserialized do not contain aggregate functions. This
logics appears to be vestigial that is no longer required, though I was
unable to pinpoint the original motivation behind it. This type of
semantic analysis should now be performed during optimization-time, so I
see no reason for it to remain.

This logic was the last remaining user of the `isAggregateVisitor` so it
has been removed.

Release note: None

#### execinfrapb: do not use tree.IndexedVarHelper

Previously, a `tree.IndexedVarHelper` was used to "rebind" indexed
variables in expressions deserialized in `execinfrapb`. The only real
function of this rebinding was to assign types to indexed variables so
that they could be type-checked. However, this was an unnecessary step
because the type checker can already determine the types of indexed vars
via the `IVarContainer` of the `tree.SemaContext`. The `exprHelper` type
already implements `tree.IndexedVarContainer` so it was already possible
to simply type-check indexed variables without rebinding them first.

This reduces allocations of `tree.IndexedVarHelper`s that can be large,
especially when there are many indexed variables.

Release note: None

#### sql/sem/tree: remove IndexedVarHelper.RebindTyped

Indexed variable rebinding is no longer needed, so it has been removed.
This reduces unnecessary allocations because `tree.IndexedVarHelper`s no
longer need to be created to perform the rebinding.

Informs #117546

Release note: None

#### sql: remove extra params for IndexedVarContainer.IndexedVarEval

This method signature for `IndexedVarContainer.IndexedVarEval` by
removing unneeded parameters. It also removes some unnecessary calls to
`Eval` on `tree.Datum`s.

Release note: None

#### sql: remove unnecessary invocation of makePredicate

Release note: None


120677: server: include WAL failover path within the stores status r=RaduBerinde a=jbowens

Expand the response from the /_status/stores endpoint to include the store's data directory and the path to the configured WAL failover secondary if configured.

Close #119795.

Epic: CRDB-35401
Release note: Adds a new field to the stores

Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com>
Co-authored-by: Jackson Owens <jackson@cockroachlabs.com>
  • Loading branch information
4 people committed Mar 19, 2024
4 parents 4582b34 + 754cac9 + fca8808 + 857d4d1 commit 471a186
Show file tree
Hide file tree
Showing 35 changed files with 190 additions and 399 deletions.
30 changes: 15 additions & 15 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -198,13 +198,13 @@
/pkg/server/multi_store* @cockroachdb/kv-prs @cockroachdb/storage
/pkg/server/node* @cockroachdb/kv-prs @cockroachdb/server-prs
/pkg/server/node_http*.go @cockroachdb/obs-inf-prs @cockroachdb/server-prs
/pkg/server/node_tenant*go @cockroachdb/obs-inf-prs @cockroachdb/multi-tenant @cockroachdb/server-prs
/pkg/server/node_tenant*go @cockroachdb/obs-inf-prs @cockroachdb/server-prs
/pkg/server/pgurl/ @cockroachdb/sql-foundations @cockroachdb/cli-prs
/pkg/server/pagination* @cockroachdb/obs-inf-prs @cockroachdb/server-prs
/pkg/server/problem_ranges*.go @cockroachdb/obs-inf-prs
/pkg/server/profiler/ @cockroachdb/obs-inf-prs @cockroachdb/kv-prs
/pkg/server/purge_auth_* @cockroachdb/obs-inf-prs @cockroachdb/server-prs
/pkg/server/server_controller_*.go @cockroachdb/multi-tenant @cockroachdb/server-prs
/pkg/server/server_controller_*.go @cockroachdb/server-prs
/pkg/server/server_controller_http.go @cockroachdb/obs-inf-prs @cockroachdb/server-prs
/pkg/server/server_controller_sql.go @cockroachdb/sql-foundations @cockroachdb/server-prs
/pkg/server/server_http*.go @cockroachdb/obs-inf-prs @cockroachdb/server-prs
Expand All @@ -215,8 +215,8 @@
/pkg/server/serverpb/authentication* @cockroachdb/obs-inf-prs @cockroachdb/prodsec @cockroachdb/server-prs
/pkg/server/serverpb/index_reco* @cockroachdb/obs-inf-prs
/pkg/server/serverrules/ @cockroachdb/obs-inf-prs @cockroachdb/server-prs
/pkg/server/settings_cache*.go @cockroachdb/multi-tenant @cockroachdb/server-prs
/pkg/server/settingswatcher/ @cockroachdb/multi-tenant @cockroachdb/server-prs
/pkg/server/settings_cache*.go @cockroachdb/server-prs
/pkg/server/settingswatcher/ @cockroachdb/server-prs
/pkg/server/span_stats*.go @cockroachdb/obs-inf-prs
/pkg/server/sql_stats*.go @cockroachdb/obs-inf-prs
/pkg/server/statement*.go @cockroachdb/obs-inf-prs
Expand All @@ -225,10 +225,10 @@
/pkg/server/status/ @cockroachdb/obs-inf-prs @cockroachdb/server-prs
/pkg/server/sticky_vfs* @cockroachdb/storage
/pkg/server/structlogging/ @cockroachdb/obs-inf-prs
/pkg/server/systemconfigwatcher/ @cockroachdb/kv-prs @cockroachdb/multi-tenant
/pkg/server/systemconfigwatcher/ @cockroachdb/kv-prs
/pkg/server/telemetry/ @cockroachdb/obs-inf-prs @cockroachdb/server-prs
/pkg/server/tenant*.go @cockroachdb/obs-inf-prs @cockroachdb/multi-tenant @cockroachdb/server-prs
/pkg/server/tenantsettingswatcher/ @cockroachdb/multi-tenant
/pkg/server/tenant*.go @cockroachdb/obs-inf-prs @cockroachdb/server-prs
/pkg/server/tenantsettingswatcher/ @cockroachdb/server-prs
/pkg/server/testserver*.go @cockroachdb/test-eng @cockroachdb/server-prs
/pkg/server/tracedumper/ @cockroachdb/obs-inf-prs @cockroachdb/server-prs
/pkg/server/user*.go @cockroachdb/obs-inf-prs @cockroachdb/server-prs @cockroachdb/prodsec
Expand Down Expand Up @@ -376,12 +376,12 @@
#!/pkg/ccl/gssapiccl/ @cockroachdb/unowned
/pkg/ccl/jwtauthccl/ @cockroachdb/cloud-identity
#!/pkg/ccl/kvccl/ @cockroachdb/kv-noreview
/pkg/ccl/kvccl/kvtenantccl/ @cockroachdb/multi-tenant
/pkg/ccl/kvccl/kvtenantccl/ @cockroachdb/server-prs
#!/pkg/ccl/upgradeccl/ @cockroachdb/release-eng
#!/pkg/ccl/logictestccl/ @cockroachdb/sql-queries-noreview
#!/pkg/ccl/sqlitelogictestccl/ @cockroachdb/sql-queries-noreview
/pkg/ccl/multiregionccl/ @cockroachdb/sql-foundations
/pkg/ccl/multitenantccl/ @cockroachdb/multi-tenant
/pkg/ccl/multitenantccl/ @cockroachdb/server-prs
/pkg/ccl/multitenant/tenantcostclient/ @cockroachdb/sqlproxy-prs
/pkg/ccl/multitenant/tenantcostserver/ @cockroachdb/sqlproxy-prs
/pkg/ccl/oidcccl/ @cockroachdb/obs-inf-prs
Expand All @@ -391,10 +391,10 @@

#!/pkg/ccl/serverccl/ @cockroachdb/unowned
/pkg/ccl/serverccl/diagnosticsccl/ @cockroachdb/obs-inf-prs
/pkg/ccl/serverccl/server_sql* @cockroachdb/multi-tenant @cockroachdb/server-prs
/pkg/ccl/serverccl/server_controller* @cockroachdb/multi-tenant @cockroachdb/server-prs
/pkg/ccl/serverccl/tenant_* @cockroachdb/multi-tenant @cockroachdb/server-prs
/pkg/ccl/serverccl/statusccl/ @cockroachdb/obs-inf-prs @cockroachdb/multi-tenant
/pkg/ccl/serverccl/server_sql* @cockroachdb/server-prs
/pkg/ccl/serverccl/server_controller* @cockroachdb/server-prs
/pkg/ccl/serverccl/tenant_* @cockroachdb/server-prs
/pkg/ccl/serverccl/statusccl/ @cockroachdb/obs-inf-prs
/pkg/ccl/serverccl/admin_* @cockroachdb/obs-inf-prs
/pkg/ccl/serverccl/api_* @cockroachdb/obs-inf-prs
/pkg/ccl/serverccl/chart_* @cockroachdb/obs-inf-prs
Expand Down Expand Up @@ -514,7 +514,7 @@
#!/pkg/keys/constants.go @cockroachdb/kv-prs-noreview
/pkg/upgrade/ @cockroachdb/release-eng
/pkg/keyvisualizer/ @cockroachdb/obs-inf-prs
/pkg/multitenant/ @cockroachdb/multi-tenant
/pkg/multitenant/ @cockroachdb/server-prs
/pkg/release/ @cockroachdb/dev-inf
/pkg/roachpb/.gitattributes @cockroachdb/dev-inf
#!/pkg/roachpb/BUILD.bazel @cockroachdb/kv-prs-noreview
Expand All @@ -533,7 +533,7 @@
/pkg/roachprod/ @cockroachdb/test-eng
/pkg/rpc/ @cockroachdb/kv-prs
/pkg/rpc/auth.go @cockroachdb/kv-prs @cockroachdb/prodsec
/pkg/rpc/auth_tenant.go @cockroachdb/multi-tenant @cockroachdb/prodsec
/pkg/rpc/auth_tenant.go @cockroachdb/server-prs @cockroachdb/prodsec
/pkg/scheduledjobs/ @cockroachdb/jobs-prs @cockroachdb/disaster-recovery
/pkg/security/ @cockroachdb/prodsec @cockroachdb/server-prs
/pkg/security/clientsecopts/ @cockroachdb/sql-foundations @cockroachdb/prodsec
Expand Down
5 changes: 0 additions & 5 deletions .github/workflows/add-issues-to-project.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,6 @@ jobs:
project-url: https://github.com/orgs/cockroachdb/projects/35
github-token: ${{ secrets.ADD_TO_PROJECT_PAT }}
labeled: T-jobs
- uses: actions/add-to-project@v0.4.0
with:
project-url: https://github.com/orgs/cockroachdb/projects/36
github-token: ${{ secrets.ADD_TO_PROJECT_PAT }}
labeled: T-multitenant
- uses: actions/add-to-project@v0.4.0
with:
project-url: https://github.com/orgs/cockroachdb/projects/45
Expand Down
4 changes: 0 additions & 4 deletions TEAMS.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,6 @@ cockroachdb/obs-inf-prs:
cockroachdb/http-api-prs: other
triage_column_id: 14196277
label: T-observability-inf
cockroachdb/multi-tenant:
# Multi-tenant team uses GH projects v2, which doesn't have a REST API, so no triage column ID
# see .github/workflows/add-issues-to-project.yml
label: T-multitenant
cockroachdb/jobs:
aliases:
cockroachdb/jobs-prs: other
Expand Down
3 changes: 3 additions & 0 deletions docs/generated/http/full.md
Original file line number Diff line number Diff line change
Expand Up @@ -4090,11 +4090,14 @@ Support status: [reserved](#support-status)
| Field | Type | Label | Description | Support status |
| ----- | ---- | ----- | ----------- | -------------- |
| store_id | [int32](#cockroach.server.serverpb.StoresResponse-int32) | | | [reserved](#support-status) |
| node_id | [int32](#cockroach.server.serverpb.StoresResponse-int32) | | | [reserved](#support-status) |
| encryption_status | [bytes](#cockroach.server.serverpb.StoresResponse-bytes) | | encryption_status is a serialized ccl/storageccl/engineccl/enginepbccl/stats.go::EncryptionStatus protobuf. | [reserved](#support-status) |
| total_files | [uint64](#cockroach.server.serverpb.StoresResponse-uint64) | | Basic file stats when encryption is enabled. Total files/bytes. | [reserved](#support-status) |
| total_bytes | [uint64](#cockroach.server.serverpb.StoresResponse-uint64) | | | [reserved](#support-status) |
| active_key_files | [uint64](#cockroach.server.serverpb.StoresResponse-uint64) | | Files/bytes using the active data key. | [reserved](#support-status) |
| active_key_bytes | [uint64](#cockroach.server.serverpb.StoresResponse-uint64) | | | [reserved](#support-status) |
| dir | [string](#cockroach.server.serverpb.StoresResponse-string) | | dir is the path to the store's data directory on the node. | [reserved](#support-status) |
| wal_failover_path | [string](#cockroach.server.serverpb.StoresResponse-string) | | wal_failover_path encodes the path to the secondary WAL directory used for failover in the event of high write latency to the primary WAL. | [reserved](#support-status) |



Expand Down
1 change: 0 additions & 1 deletion pkg/cmd/roachtest/registry/owners.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ const (
OwnerStorage Owner = `storage`
OwnerTestEng Owner = `test-eng`
OwnerDevInf Owner = `dev-inf`
OwnerMultiTenant Owner = `multi-tenant`
OwnerClusterObs Owner = `cluster-observability`
)

Expand Down
10 changes: 4 additions & 6 deletions pkg/cmd/roachtest/tests/acceptance.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,6 @@ func registerAcceptance(r registry.Registry) {
{name: "cluster-init", fn: runClusterInit},
{name: "rapid-restart", fn: runRapidRestart},
},
registry.OwnerMultiTenant: {
{
name: "multitenant",
fn: runAcceptanceMultitenant,
},
},
registry.OwnerObsInf: {
{name: "status-server", fn: runStatusServer},
},
Expand All @@ -83,6 +77,10 @@ func registerAcceptance(r registry.Registry) {
fn: runAcceptanceClusterReplication,
numNodes: 3,
},
{
name: "multitenant",
fn: runAcceptanceMultitenant,
},
},
registry.OwnerSQLFoundations: {
{
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/roachtest/tests/multitenant_shared_process.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func registerMultiTenantSharedProcess(r registry.Registry) {

r.Add(registry.TestSpec{
Name: "multitenant/shared-process/basic",
Owner: registry.OwnerMultiTenant,
Owner: registry.OwnerDisasterRecovery,
Cluster: r.MakeClusterSpec(crdbNodeCount + 1),
Leases: registry.MetamorphicLeases,
CompatibleClouds: registry.AllExceptAWS,
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/roachtest/tests/multitenant_upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func registerMultiTenantUpgrade(r registry.Registry) {
Cluster: r.MakeClusterSpec(2),
CompatibleClouds: registry.AllExceptAWS,
Suites: registry.Suites(registry.Nightly),
Owner: registry.OwnerMultiTenant,
Owner: registry.OwnerDisasterRecovery,
NonReleaseBlocker: false,
Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
runMultiTenantUpgrade(ctx, t, c, t.BuildVersion())
Expand Down
4 changes: 0 additions & 4 deletions pkg/internal/team/TEAMS.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,6 @@ cockroachdb/obs-inf-prs:
cockroachdb/http-api-prs: other
triage_column_id: 14196277
label: T-observability-inf
cockroachdb/multi-tenant:
# Multi-tenant team uses GH projects v2, which doesn't have a REST API, so no triage column ID
# see .github/workflows/add-issues-to-project.yml
label: T-multitenant
cockroachdb/jobs:
aliases:
cockroachdb/jobs-prs: other
Expand Down
5 changes: 5 additions & 0 deletions pkg/roachpb/metadata.proto
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,11 @@ message StoreProperties {
optional bool encrypted = 1 [(gogoproto.nullable) = false];
// read_only indicates whether the store is attached read_only.
optional bool read_only = 2 [(gogoproto.nullable) = false];
// dir is the path to the store's data directory on the node.
optional string dir = 4 [(gogoproto.nullable) = false];
// wal_failover_path encodes the path to the secondary WAL directory used for
// failover in the event of high write latency to the primary WAL.
optional string wal_failover_path = 5;

// disk_properties reports details about the underlying filesystem,
// when the store is supported by a file store. Unset otherwise.
Expand Down
10 changes: 10 additions & 0 deletions pkg/server/serverpb/status.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1598,6 +1598,11 @@ message StoreDetails {
(gogoproto.casttype) =
"github.com/cockroachdb/cockroach/pkg/roachpb.StoreID"
];
int32 node_id = 7 [
(gogoproto.customname) = "NodeID",
(gogoproto.casttype) =
"github.com/cockroachdb/cockroach/pkg/roachpb.NodeID"
];

// TODO(mberhault): add a lot more information about stores. eg:
// - path
Expand All @@ -1615,6 +1620,11 @@ message StoreDetails {
// Files/bytes using the active data key.
uint64 active_key_files = 5;
uint64 active_key_bytes = 6;
// dir is the path to the store's data directory on the node.
string dir = 8;
// wal_failover_path encodes the path to the secondary WAL directory used for
// failover in the event of high write latency to the primary WAL.
string wal_failover_path = 9 [(gogoproto.nullable) = true];
}

message StoresResponse {
Expand Down
29 changes: 15 additions & 14 deletions pkg/server/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -3748,25 +3748,26 @@ func (s *systemStatusServer) Stores(

resp := &serverpb.StoresResponse{}
err = s.stores.VisitStores(func(store *kvserver.Store) error {
storeDetails := serverpb.StoreDetails{
StoreID: store.Ident.StoreID,
}

envStats, err := store.TODOEngine().GetEnvStats()
eng := store.TODOEngine()
envStats, err := eng.GetEnvStats()
if err != nil {
return err
}

if len(envStats.EncryptionStatus) > 0 {
storeDetails.EncryptionStatus = envStats.EncryptionStatus
props := eng.Properties()
storeDetails := serverpb.StoreDetails{
StoreID: store.Ident.StoreID,
NodeID: nodeID,
EncryptionStatus: envStats.EncryptionStatus,
TotalFiles: envStats.TotalFiles,
TotalBytes: envStats.TotalBytes,
ActiveKeyFiles: envStats.ActiveKeyFiles,
ActiveKeyBytes: envStats.ActiveKeyBytes,
Dir: props.Dir,
}
if props.WalFailoverPath != nil {
storeDetails.WalFailoverPath = *props.WalFailoverPath
}
storeDetails.TotalFiles = envStats.TotalFiles
storeDetails.TotalBytes = envStats.TotalBytes
storeDetails.ActiveKeyFiles = envStats.ActiveKeyFiles
storeDetails.ActiveKeyBytes = envStats.ActiveKeyBytes

resp.Stores = append(resp.Stores, storeDetails)

return nil
})
if err != nil {
Expand Down
6 changes: 1 addition & 5 deletions pkg/sql/catalog/schemaexpr/computed_exprs.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
package schemaexpr

import (
"context"

"github.com/cockroachdb/cockroach/pkg/sql/catalog"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
Expand All @@ -35,9 +33,7 @@ type RowIndexedVarContainer struct {
var _ eval.IndexedVarContainer = &RowIndexedVarContainer{}

// IndexedVarEval implements eval.IndexedVarContainer.
func (r *RowIndexedVarContainer) IndexedVarEval(
ctx context.Context, idx int, e tree.ExprEvaluator,
) (tree.Datum, error) {
func (r *RowIndexedVarContainer) IndexedVarEval(idx int) (tree.Datum, error) {
rowIdx, ok := r.Mapping.Get(r.Cols[idx].GetID())
if !ok {
return tree.DNull, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/colexec/colexecargs/expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,5 @@ func (h *ExprHelper) ProcessExpr(
if expr.LocalExpr != nil {
return expr.LocalExpr, nil
}
return execinfrapb.DeserializeExprWithTypes(ctx, expr.Expr, typs, h.SemaCtx, evalCtx)
return execinfrapb.DeserializeExpr(ctx, expr, typs, h.SemaCtx, evalCtx)
}
6 changes: 2 additions & 4 deletions pkg/sql/colexec/colexectestutils/proj_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,8 @@ type MockTypeContext struct {
var _ eval.IndexedVarContainer = &MockTypeContext{}

// IndexedVarEval implements the eval.IndexedVarContainer interface.
func (p *MockTypeContext) IndexedVarEval(
ctx context.Context, idx int, e tree.ExprEvaluator,
) (tree.Datum, error) {
return tree.DNull.Eval(ctx, e)
func (p *MockTypeContext) IndexedVarEval(idx int) (tree.Datum, error) {
return tree.DNull, nil
}

// IndexedVarResolvedType implements the tree.IndexedVarContainer interface.
Expand Down
3 changes: 0 additions & 3 deletions pkg/sql/distsql_plan_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -504,9 +504,6 @@ func (dsp *DistSQLPlanner) createStatsPlan(

var rb renderBuilder
rb.init(exec.Node(planNode(&scan)), exec.OutputOrdering{})
for i, expr := range exprs {
exprs[i] = rb.r.ivarHelper.RebindTyped(expr)
}
rb.setOutput(exprs, resultCols)

err = dsp.createPlanForRender(ctx, p, rb.r, planCtx)
Expand Down
1 change: 0 additions & 1 deletion pkg/sql/execinfrapb/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ go_library(
"//pkg/sql/rowenc",
"//pkg/sql/sem/eval",
"//pkg/sql/sem/normalize",
"//pkg/sql/sem/transform",
"//pkg/sql/sem/tree",
"//pkg/sql/sem/tree/treewindow",
"//pkg/sql/sessiondata",
Expand Down

0 comments on commit 471a186

Please sign in to comment.