cockroachdb / cockroachdb/cockroach

admission: FK cascade deletes from TTL bypass elastic CPU control due to locking priority adjustment

Open
#166,192 2 comments 0 reactions 0 assignees View on GitHub
A-admission-control A-kv branch-master C-bug T-kv
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

## Summary

FK cascade deletes triggered by TTL are not subject to elastic CPU admission control. This is because cascade deletes (post-queries) run after the parent DELETE has acquired locks, causing `AdjustedPriorityWhenHoldingLocks` to bump the admission priority from `BulkLowPri` (-100) to `NormalPri` (0) and then to `LockingNormalPri` (10), which is above the elastic CPU threshold of `BulkNormalPri` (-30).

This means a TTL job that triggers a cascade delete on a child table can saturate CPU and bypass elastic CPU pacing.

Relates to TREQ-1596.

## How TTL sets low priority

TTL creates a fresh transaction per delete batch with `BulkLowPri` priority and runs the DELETE via the internal executor with `BulkLowQoS` session QoS:

- [`ttljob_processor.go:437-438`](https://github.com/cockroachdb/cockroach/blob/d608df0444b/pkg/sql/ttl/ttljob/ttljob_processor.go#L437-L438) — transaction created with `BulkLowPri`
- [`ttljob_query_builder.go:283-287`](https://github.com/cockroachdb/cockroach/blob/d608df0444b/pkg/sql/ttl/ttljob/ttljob_query_builder.go#L283-L287) — DELETE executed with `BulkLowQoS`

## Why cascades bypass elastic CPU

1. The parent TTL DELETE runs first, acquiring locks on the rows it deletes. This makes the transaction "locking" (`IsLocking() = true`).

2. FK cascade deletes run as post-queries **after** the parent DELETE completes, within the same transaction:
- [`distsql_running.go:2854`](https://github.com/cockroachdb/cockroach/blob/d608df0444b/pkg/sql/distsql_running.go#L2854) — cascade runs via `dsp.Run` using `planner.txn`

3. When the cascade's scan operator initializes, it calls `txn.AdmissionHeader()`:
- Vectorized engine: [`cfetcher.go:567`](https://github.com/cockroachdb/cockroach/blob/d608df0444b/pkg/sql/colfetcher/cfetcher.go#L567)
- Row engine: [`kv_batch_fetcher.go:436`](https://github.com/cockroachdb/cockroach/blob/d608df0444b/pkg/sql/row/kv_batch_fetcher.go#L436)

4. `Txn.AdmissionHeader()` checks `IsLocking()`, which is now true, and calls `AdjustedPriorityWhenHoldingLocks`:
- [`txn.go:1927-1934`](https://github.com/cockroachdb/cockroach/blob/d608df0444b/pkg/kv/txn.go#L1927-L1934)
- [`admissionpb.go:160-170`](https://github.com/cockroachdb/cockroach/blob/d608df0444b/pkg/util/admission/admissionpb/admissionpb.go#L160-L170) — bumps any priority below `NormalPri` up to `NormalPri`

5. The elastic CPU check requires `priority <= BulkNormalPri` (-30), but the adjusted priority is `LockingNormalPri` (10), so **elastic CPU control is not applied**:
- KV server: [`kvadmission.go:396`](https://github.com/cockroachdb/cockroach/blob/d608df0444b/pkg/kv/kvserver/kvadmission/kvadmission.go#L396)
- SQL read pacer: [`cfetcher.go:567`](https://github.com/cockroachdb/cockroach/blob/d608df0444b/pkg/sql/colfetcher/cfetcher.go#L567)

## Scope

This priority bump is not cascade-specific — it affects any work done after the TTL DELETE acquires locks within the same transaction. However:

- **Without FKs**: The TTL DELETE batch is created and sent before the transaction becomes locking. No priority bump occurs.
- **With outbound FKs (no cascade)**: FK constraint checks run after locks are acquired, but these are small point lookups with minimal impact.
- **With inbound FK CASCADE**: The cascade scan + cascade delete both run after locks are acquired. If the child table lacks an index on the FK column, this results in a **full table scan at elevated priority**, which can saturate CPU.

### Experimental verification

Added a warning log to `AdmissionHeader()` when the priority is adjusted. Results from existing TTL unit tests:

| Test | FK configuration | `bulk-low-pri` adjustments |
|------|-----------------|---------------------------|
| `TestRowLevelTTLNoTestingKnobs` | No FKs | 0 |
| `TestOutboundForeignKey` | Outbound FK | 8 |
| `TestInboundForeignKeyOnDeleteCascade` | Inbound FK CASCADE | 18 |

## Tradeoff

As discussed with @sumeerbhola, the locking priority adjustment exists to avoid lock priority inversion — without it, foreground queries blocked on TTL-held locks would be unnecessarily queued behind other low-priority work in admission control. Removing the adjustment would fix the elastic CPU bypass but introduce priority inversion problems when foreground queries contend with TTL locks.

The only way to avoid this tradeoff entirely would be to reduce the rate at which TTL issues delete transactions (before lock acquisition), but we don't currently have a resource-aware mechanism for that.

See the detailed discussion of the priority inversion problem in [`intentresolver/admission.go`](https://github.com/cockroachdb/cockroach/blob/d608df0444b/pkg/kv/kvserver/intentresolver/admission.go).

Jira issue: CRDB-61789

Epic CRDB-62104

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.