cockroachdb / cockroachdb/cockroach

obs/ash: TxnQueryWait samples missing workload attribution

Closed
#167,155 1 comment 0 reactions 1 assignee Claimed by @alyshanjahani-crl View on GitHub
A-kv branch-master C-bug O-agent T-observability v26.3.0-prerelease
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

**Describe the problem**

ASH samples for `TxnQueryWait` lock wait events have empty `workload_id`, while the closely related `TxnPushWait` events correctly show `workload_id = "INTENT_RESOLUTION"`. This makes it difficult to attribute a significant source of lock wait time in contended workloads.

**To Reproduce**

1. Run a TPC-C `--no-wait` workload with 100 warehouses (heavy intent contention)
2. Query ASH:
```sql
SELECT work_event_type, work_event, workload_id, count(*) AS sample_count
FROM crdb_internal.cluster_active_session_history
WHERE sample_time > now() - INTERVAL '10 minutes'
GROUP BY work_event_type, work_event, workload_id
ORDER BY sample_count DESC
LIMIT 10;
```
3. Observe that `TxnPushWait` has `workload_id = INTENT_RESOLUTION` but `TxnQueryWait` has empty `workload_id`:
```
work_event_type | work_event | workload_id | sample_count
------------------+----------------------+-------------------+---------------
LOCK | TxnPushWait | INTENT_RESOLUTION | 8052
LOCK | TxnQueryWait | | 6990
```

**Expected behavior**

`TxnQueryWait` samples should be attributed to `INTENT_RESOLUTION` (or the appropriate originating workload), since TxnQueryWait is a follow-up to TxnPushWait — the txnwait queue periodically issues QueryTxn requests to poll pushee transaction status.

**Root cause**

All production PushTxn requests flow through the intent resolver's `MaybePushTransactions`, which sets workload attribution on the batch header ([intent_resolver.go:466-467](https://github.com/cockroachdb/cockroach/blob/master/pkg/kv/kvserver/intentresolver/intent_resolver.go#L466-L467)):
```go
b.Header.WorkloadID = uint64(workloadid.WORKLOAD_ID_INTENT_RESOLUTION)
b.Header.WorkloadType = workloadid.WorkloadTypeSystem.ToUint32()
```

This covers multiple callers:
- **Lock table waiter** (`pushLockTxn` → `w.ir.PushTransaction` → `MaybePushTransactions`): when a transaction encounters a conflicting lock during LockWait, it pushes the lock holder through the intent resolver.
- **Async intent resolution** (`CleanupIntentsAsync` / `CleanupTxnIntentsAsync`): resolving intents discovered during reads/writes.
- **Rangefeed txn pusher** (`replica_rangefeed.go`): also goes through `MaybePushTransactions`.

The one exception is the **range merge waiter** (`replica.go:2596`), which creates its own PushTxn batch without workload attribution, but this is a rare path.

When PushTxn arrives at the concurrency manager ([concurrency_manager.go:458-479](https://github.com/cockroachdb/cockroach/blob/master/pkg/kv/kvserver/concurrency/concurrency_manager.go#L458-L479)), it extracts the workload info and sets the ASH work state for `TxnPushWait`.

However, the txnwait queue only receives the individual `*kvpb.PushTxnRequest` — the batch header is not passed through. When the queue issues QueryTxn requests in [`queryTxnStatus()`](https://github.com/cockroachdb/cockroach/blob/master/pkg/kv/kvserver/txnwait/queue.go#L1046-L1086), it creates a new `kv.Batch` that only sets `Timestamp` — no `WorkloadID` or `WorkloadType`. This new batch arrives at the concurrency manager with zero-valued workload fields, resulting in empty `workload_id` in ASH.

**Suggested fix**

Set `INTENT_RESOLUTION` workload attribution on the batch in `queryTxnStatus()` (`pkg/kv/kvserver/txnwait/queue.go`):

```go
b.Header.WorkloadID = uint64(workloadid.WORKLOAD_ID_INTENT_RESOLUTION)
b.Header.WorkloadType = workloadid.WorkloadTypeSystem.ToUint32()
```

A more precise alternative would be to thread the workload info from the original PushTxn request through the queue (e.g., store on `pendingTxn` or `waitingPush`) and apply it when issuing QueryTxn. However, hardcoding `INTENT_RESOLUTION` is reasonable since nearly all PushTxn requests already get this label via `MaybePushTransactions` — including pushes originating from lock contention (LockWait → `pushLockTxn`), not just "intent resolution" in the narrow sense. The `INTENT_RESOLUTION` label is already used broadly to cover all lock/intent conflict resolution.

Jira issue: CRDB-62263

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.