cockroachdb / cockroachdb/cockroach

tenantcapabilitieswatcher: placeholder entries from getInternal are never garbage-collected

Open
#170,632 1 comment 0 reactions 0 assignees View on GitHub
branch-master C-bug T-db-server
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

## Summary

In `pkg/multitenant/tenantcapabilities/tenantcapabilitieswatcher/watcher.go`, when `getInternal` is called for a tenant ID not yet seen by the rangefeed, it creates a placeholder entry (`Entry: nil`) in `w.mu.store` but does not add a corresponding entry to `w.mu.lastUpdate`. Since `removeEntriesBeforeTimestamp` — the only cleanup path on rangefeed restart — only iterates `w.mu.lastUpdate`, these placeholders are never garbage-collected.

## Details

`getInternal` (called by `GetInfo`/`GetCapabilities`) creates placeholders like this:

```go
cp = &watcherEntry{
Entry: nil,
changeCh: make(chan struct{}),
}
w.mu.store[tenantID] = cp
```

This adds to `store` but NOT to `lastUpdate`.

On rangefeed restart, `removeEntriesBeforeTimestamp` cleans up stale entries:

```go
for tid, prevTs := range w.mu.lastUpdate {
if prevTs.Less(ts) {
w.removeEntryForTenantIDLocked(ctx, tid)
}
}
```

Since placeholders have no `lastUpdate` entry, they are skipped entirely. They persist in `store` until a real rangefeed event arrives for that tenant ID — which may never happen if the tenant doesn't exist.

**Contrast with tenant settings watcher**: `tenantsettingswatcher.overridesStore` uses the same "placeholder on read" pattern (`getTenantOverrides`), but its `setAll` method does a complete map replacement on each full rangefeed scan, naturally purging stale placeholders. The capabilities watcher does incremental cleanup instead, so placeholders slip through.

**Current impact**: Low. Tenant IDs are bounded and each placeholder is small (~pointer + channel). However, if `GetInfo` is called with many non-existent tenant IDs (e.g., via auth probes), placeholders accumulate indefinitely with no cleanup path.

## Suggested Fix

Either:
1. Add placeholder tenant IDs to `lastUpdate` with a zero timestamp so they get cleaned up on restart, or
2. In `removeEntriesBeforeTimestamp`, also iterate `store` and remove entries that have no corresponding `lastUpdate` entry (i.e., placeholders).

Epic: none

Jira issue: CRDB-64100

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.