cockroachdb / cockroachdb/cockroach
sql/catalog: descriptor lease refcount and monitor bytes leaked on error and panic paths
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Summary:**
Two paths in the descriptor lease machinery leak a lease refcount or charge a node-lifetime monitor when a panic or an early error return passes through them. A leaked lease refcount is not bounded by lease duration: `CountLeases` filters on `sql_liveness_is_alive`, and the two-version-invariant waits retry unboundedly, so the effect is a DDL stall that persists until the node restarts. Part of #174944.
**Findings:**
- `descs.leasedDescriptors.getResult`: `ldesc` (`lease.LeasedDescriptor`) — the descriptor's refcount is already incremented, but it is registered with the collection only at `cache.Upsert`. Two intervening branches return with it still held (the `retryOnModifiedDescriptor` path and an `AssertionFailedf` path), so it leaks on ordinary errors today as well as on a panic.
- `lease.storage.upsertLeaseLocked`: `boundAccount` — `Grow(descState.getByteSize())`, `active.insert` and `names.insert` are consecutive plain statements. A panic between the grow and the insert charges bytes to the `leased-descriptors` monitor, which is node-lifetime with an unlimited local limit, so nothing ever releases them. `lease.insertDescriptorVersions` in the same package is the model: it uses a deferred shrink disarmed on success.
**Next Steps:**
- [ ] Defer a release of `ldesc` in `getResult`, disarmed once `Upsert` succeeds
- [ ] Adopt the `insertDescriptorVersions` deferred-shrink idiom in `upsertLeaseLocked`
Epic: none
Jira issue: CRDB-68123
Epic CRDB-65516
Contributor guide
Research direction
Locate descs.leasedDescriptors.getResult and lease.storage.upsertLeaseLocked, then read lease.insertDescriptorVersions as the deferred-shrink model. Ensure the ldesc release remains deferred until Upsert succeeds and the boundAccount charge is undone unless insertion completes, covering early errors and panic paths.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100