cockroachdb / cockroachdb/cockroach
obs/ash: add store_id to ASH samples
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Is your feature request related to a problem? Please describe.**
Active Session History (ASH) samples currently record `node_id` but not `store_id`. For KV-layer work events (e.g. `KVEval`, `ReplicaSend`, `LatchWait`, `LockWait`, `RaftProposalWait`), knowing which store the work occurred on would help attribute load and contention to specific stores — useful on multi-store nodes where a single `node_id` is too coarse.
**Describe the solution you'd like**
Add a `store_id` column to ASH samples, populated for KV-server instrumentation points that operate on a replica.
**Important:** this is *not* a simple parallel of `node_id`. `node_id` is a property of the process-wide sampler singleton — it's set once at init ([`sampler.go:166`](https://github.com/cockroachdb/cockroach/blob/b4c850ad69766fc7f9acdda105e35180a7f66713/pkg/obs/ash/sampler.go#L166)) and stamped unconditionally on every sample ([`sampler.go:307`](https://github.com/cockroachdb/cockroach/blob/b4c850ad69766fc7f9acdda105e35180a7f66713/pkg/obs/ash/sampler.go#L307)). A store sits *below* a node (one node can host multiple stores), so `store_id` is a property of the per-goroutine `WorkState`, not the sampler, and is only meaningful for a subset of instrumentation points.
Of the 24 instrumentation points ([`ash_instrumentation_points.txt`](https://github.com/cockroachdb/cockroach/blob/b4c850ad69766fc7f9acdda105e35180a7f66713/pkg/obs/ash/ash_instrumentation_points.txt)):
- **Have a store** (all `pkg/kv/kvserver/...`, run on a `Replica`): `KVEval`, `ReplicaSend`, `LatchWait`, `LockWait`, `RaftProposalWait`, `LeaseAcquisition`, `Backpressure`, `CommitWaitSleep`, `TxnPushWait`, `TxnQueryWait`, `TenantRateLimit`, `ReplicationFlowControl`.
- **No store concept** (`store_id` would be NULL): `Optimize`, `DistSenderLocal`/`DistSenderRemote`, `OutboxSend`/`InboxRecv`, `BatchFlowCoordinator`, `ColExecSync`, execinfra processors, admission `work_queue`.
So unlike `node_id` (always populated), `store_id` would be **sparsely populated**.
**Work involved:**
_Mechanical / output-side (mirrors `node_id`, ~easy):_
- [ ] Add `store_id` to the `ASHSample` proto ([`status.proto:1221`](https://github.com/cockroachdb/cockroach/blob/b4c850ad69766fc7f9acdda105e35180a7f66713/pkg/server/serverpb/status.proto#L1221)) and Go struct ([`types.go:67`](https://github.com/cockroachdb/cockroach/blob/b4c850ad69766fc7f9acdda105e35180a7f66713/pkg/obs/ash/types.go#L67))
- [ ] Map it in the RPC handler (`ListLocalActiveSessionHistory` in `pkg/server/status.go`)
- [ ] Add the column + `addRow` value to both virtual tables ([`crdb_internal.go:10000`](https://github.com/cockroachdb/cockroach/blob/b4c850ad69766fc7f9acdda105e35180a7f66713/pkg/sql/crdb_internal.go#L10000), [`crdb_internal.go:10061`](https://github.com/cockroachdb/cockroach/blob/b4c850ad69766fc7f9acdda105e35180a7f66713/pkg/sql/crdb_internal.go#L10061)) and the two `information_schema` views
_Input-side (the real work):_
- [ ] Add a `StoreID roachpb.StoreID` field to `WorkloadInfo` ([`types.go:50`](https://github.com/cockroachdb/cockroach/blob/b4c850ad69766fc7f9acdda105e35180a7f66713/pkg/obs/ash/types.go#L50)), defaulting to 0 (unset). This **avoids any `SetWorkState` signature change** — existing call sites are untouched; only KV call sites add `StoreID: ...`. The value flows automatically via the existing `state.WorkloadInfo = info` assignment in `SetWorkState`.
- [ ] At each KV call site, fetch the store ID from the `Replica` and set it in the `WorkloadInfo` literal (e.g. [`replica_send.go:114`](https://github.com/cockroachdb/cockroach/blob/b4c850ad69766fc7f9acdda105e35180a7f66713/pkg/kv/kvserver/replica_send.go#L114))
- [ ] Copy `StoreID` into the emitted `ASHSample` in `takeSample` ([`sampler.go:305`](https://github.com/cockroachdb/cockroach/blob/b4c850ad69766fc7f9acdda105e35180a7f66713/pkg/obs/ash/sampler.go#L305))
- [ ] Decide semantics for no-store sites (leave 0, surface as NULL)
**Describe alternatives you've considered**
- _Status quo:_ leaving attribution at `node_id` granularity. Adequate for single-store nodes but loses information on multi-store deployments.
- _Dedicated `WorkState.StoreID` field:_ semantically cleaner than adding to `WorkloadInfo` (which otherwise groups workload-*identity* fields), but requires a new `SetWorkState` parameter, changing all ~24 call sites. The `WorkloadInfo` route is preferred for minimal churn.
**Additional context**
Effort is dominated by populating `store_id` at the KV-server call sites, not by the proto/table changes. The output-side plumbing is a direct copy of the existing `node_id` pattern.
Jira issue: CRDB-64435
Contributor guide
Assessment
This issue has not been assessed yet.