cockroachdb / cockroachdb/cockroach

obs/ash: work states from secondary virtual clusters are attributed to the system tenant

Open
#175,053 1 comment 0 reactions 0 assignees View on GitHub
A-cluster-observability A-multitenancy C-bug O-agent T-observability
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

**Describe the problem**

Several `ash.SetWorkState` call sites pass a `TenantID` that is unset. [`setWorkState`](https://github.com/cockroachdb/cockroach/blob/master/pkg/obs/ash/work_state.go#L133) silently coerces an unset tenant to the system tenant:

```go
if !tenantID.IsSet() { tenantID = roachpb.SystemTenantID }
```

As a result, work performed on behalf of a secondary virtual cluster is recorded under tenant 1. Because the ASH read path filters samples by tenant, those samples are **missing** from the secondary tenant's `crdb_internal.node_active_session_history` and instead show up under the system tenant.

Two independent sources:

1. [`kvcoord/transport.go:209`](https://github.com/cockroachdb/cockroach/blob/master/pkg/kv/kvclient/kvcoord/transport.go#L209) uses `roachpb.ClientTenantFromContext(ctx)` for the `DistSenderLocal`/`DistSenderRemote` work states. That value is documented as "the tenant that's the client of an RPC" and is populated only by the **server-side** auth interceptor. A DistSender is the *client*, so it is always unset there.
2. [`flowinfra/inbound.go:146`](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/flowinfra/inbound.go#L146) and [`colrpc/inbox.go:369`](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/colflow/colrpc/inbox.go#L369) use `admissionInfo.TenantID` for `InboxRecv`, which is likewise unset for inbound DistSQL streams.

KV-server-side call sites (`KVEval`, `ReplicaSend`) are unaffected, since the interceptor *does* populate the context there.

**To Reproduce**

1. Start a cluster and create a shared-process virtual cluster:
```sql
CREATE VIRTUAL CLUSTER shared1;
ALTER VIRTUAL CLUSTER shared1 START SERVICE SHARED;
SET CLUSTER SETTING obs.ash.sample_interval = '500ms';
```
2. Connect **to `shared1`**, and run a scan-heavy DistSQL workload in a session with a distinctive marker:
```sql
SET application_name = 'ASHREPRO';
CREATE TABLE repro(k INT PRIMARY KEY, v STRING);
INSERT INTO repro SELECT g, repeat('x',200) FROM generate_series(1,400000) g;
-- then repeatedly, from several concurrent sessions:
SELECT count(*), sum(length(v)) FROM repro;
SELECT count(*) FROM repro WHERE k % 7 = 0;
```
3. From the **system** tenant:
```sql
SELECT work_event, tenant_id, count(*) FROM crdb_internal.node_active_session_history
WHERE app_name = 'ASHREPRO' GROUP BY 1,2 ORDER BY 3 DESC;
```
```
work_event | tenant_id | count
BatchFlowCoordinator| 3 | 3245
ColExecSync | 3 | 1598
KVEval | 3 | 1512
InboxRecv | 1 | 264 <-- system tenant never ran this SQL
ReplicaSend | 3 | 17
DistSenderLocal | 1 | 4 <--
Optimize | 3 | 3
DistSenderRemote | 1 | 1 <--
```
`app_name='ASHREPRO'` is carried on the batch header independently of `TenantID`, so these rows are self-contradictory: SQL only `shared1` ran, attributed to tenant 1.

4. Run the **same query from `shared1`** -- the three mis-attributed events are absent, while every other count is identical:
```
BatchFlowCoordinator| 3 | 3245
ColExecSync | 3 | 1598
KVEval | 3 | 1512
ReplicaSend | 3 | 17
Optimize | 3 | 3
```

**Expected behavior**

Work performed for a secondary virtual cluster should carry that tenant's ID, so it appears in that tenant's `crdb_internal.node_active_session_history` and not under the system tenant.

**Additional context**

Impact is twofold: a secondary tenant loses visibility into its own network-wait work (`InboxRecv`, `DistSender*` -- exactly the events that explain distributed-query latency), and the system tenant's ASH is polluted with other tenants' work. The loss is silent -- no metric counts the coerced samples.

Each SQL server already constructs its own DistSender, so a per-DistSender tenant identity would be correct for (1) in all deployment shapes. Independently, the coercion at `work_state.go:133` is what turns an attribution bug into invisible data loss and would be worth making non-silent.

**Environment:** master (v26.4.0-alpha), Linux/amd64, `cockroach sql`

Jira issue: CRDB-68195

Contributor guide

Open the contributing guide

Research direction

Start with pkg/obs/ash/work_state.go:setWorkState, then inspect the call sites in pkg/kv/kvclient/kvcoord/transport.go, pkg/sql/flowinfra/inbound.go, and pkg/sql/colflow/colrpc/inbox.go. Reproduce the issue using the shared virtual-cluster workload and ASH queries described above. Done means secondary-tenant work appears under its own tenant and is no longer attributed to the system tenant.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, sql
Domain
databases, distributed-systems, observability-sre
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.