cockroachdb / cockroachdb/cockroach

kvserver: tolerate legacy orphaned range-local keys in startup invariant

Open
#173,616 4 comments 0 reactions 1 assignee Assigned to @natakhot271 View on GitHub
A-kv C-enhancement P-3 T-kv
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

## Problem

The test-build startup invariant in `iterateRangeDescriptorsFromDiskHelper` ([init.go:328](https://github.com/cockroachdb/cockroach/blob/8987f01ffdf/pkg/kv/kvserver/kvstorage/init.go#L328)) currently fails hard when it detects orphaned range-local keys (keys outside any known range descriptor). This assertion was added to catch bugs, but it doesn't distinguish between:

1. **Legacy corruption** - orphaned keys created by pre-25.4 binaries that lacked the fix from #73462 (commit `1ca06e49622`)
2. **New bugs** - orphaned keys that shouldn't exist if the store only ran 25.4+

This causes test failures in mixed-version scenarios where stores ran older binaries (e.g., #173559).

## Current Behavior

```go
// In iterateRangeDescriptorsFromDiskHelper
if performInvariantChecks && key.Compare(curRangeStart) < 0 {
log.Fatalf("range local key %s outside of a known range", key)
}
```

**Result:** Any orphaned key causes test builds to refuse to start, even if it's known legacy corruption from a pre-fix binary. Even when the fix (commit `1ca06e49622`) gets backported to 25.3-, it will only be in the patch-release versions like 25.3.x+1, which is why we need the tolerance backported as well for pre-patch cluster upgrades.

## Desired Behavior

The startup invariant should:
- **Allow startup** if the store's persisted cluster version indicates it ran pre-25.4 binaries (which are known to leak orphaned keys via #73462)
- **Log a loud warning** about the legacy corruption for visibility
- **Still fail hard** if the store has only ever run 25.4+ (where such keys indicate a new bug)

This preserves the invariant's bug-catching power while tolerating known historical corruption.

## Proposed Implementation

```go
func iterateRangeDescriptorsFromDiskHelper(...) error {
storeVersion := getPersistedClusterVersion(engine) // or similar

for iter.Next() {
key := iter.Key()

if key.Compare(curRangeStart) < 0 {
// Found orphaned range-local key

if performInvariantChecks {
if storeVersion.Less(clusterversion.V25_4_Start) {
// Store ran pre-fix versions - legacy corruption expected
log.Warningf(ctx,
"LEGACY CORRUPTION: range local key %s outside of known range "+
"(store ran %s which had snapshot bug #73462, tolerating for startup)",
key, storeVersion)
iter.NextKey()
continue
} else {
// Store only ran v25.4+ which has the fix - new bug!
return errors.AssertionFailedf(
"range local key %s outside of a known range "+
"(store has only run v25.4+ which should never create these)",
key)
}
}

// Production builds: always skip (no invariant check)
iter.NextKey()
}
// ... rest of iteration
}
}
```

## Scope

**In scope:**
- Add version check to the startup invariant
- Downgrade to warning for stores with persisted version < 25.4
- Maintain hard failure for stores that only ran 25.4+
- Add test coverage for both code paths
- **Backport to all active releases** (25.4.x, 26.2.x, 26.3.x (no label yet)) - these releases have the strict startup assertion and can encounter stores with legacy orphans from mixed-version upgrade tests

**Why backport is needed:**

How releases WITH the fix still encounter orphaned keys:

1. A mixed-version test runs some nodes at v25.3 (or earlier), which has the
orphan-creating bug (#73462).
2. The bug fires — orphaned range-local keys are written to disk on one of
those nodes.
3. The test upgrades that node to 25.4 (or 26.2, or 26.3). The new binary
starts up and runs the startup invariant scan.
4. The strict assertion performInvariantChecks sees the orphaned keys and fatals. The node refuses
to start.

**Out of scope:**
- Backporting the fix (tracked in #173559)
- Telemetry/observability for production (tracked separately)
- Cleaning up the orphaned keys (they're inert and self-heal)
- Test framework error message improvements (tracked separately)

## Testing

1. **Unit test:** Create a store with orphaned keys + persisted version 25.3 → startup should succeed with warning
2. **Unit test:** Create a store with orphaned keys + persisted version 25.4 → startup should fail with assertion:
- The store claims it has only ever run 25.4+
- 25.4+ has the fix that prevents creating orphans
- Therefore orphans shouldn't exist
3. **Integration test:** Verify mixed-version roachtest no longer fails on legacy orphans

## Success Criteria

- [ ] Startup invariant distinguishes legacy vs new corruption by store version
- [ ] Test builds can start on stores with known legacy orphans (with loud warning)
- [ ] Test builds still catch new orphans on fresh 25.4+ stores
- [ ] Mixed-version tests (25.3→25.4) pass in test builds

## Context

This issue arose from #173559, where a mixed-version test (`v25.3.7 → release-25.4`) failed because:
1. The v25.3.7 binary applied a narrowing snapshot and leaked orphaned keys (known bug #73462)
2. The release-25.4 assertions build restarted and detected the orphan
3. The invariant failed hard, not recognizing this as expected legacy corruption

The root cause is fixed by `1ca06e49622` (present in 25.4+, not being backported to
prior releases). This issue adds tolerance for the window where stores may have run unfixed binaries.

**Related:**
- #73462 - Root cause (narrowing snapshot leak)
- #173559 - Test failure that exposed the need for tolerance

Epic: none

Jira issue: CRDB-66928

Contributor guide

Open the contributing guide

Research direction

Start in pkg/kv/kvserver/kvstorage/init.go at iterateRangeDescriptorsFromDiskHelper and its invariant check around line 328. Trace how the persisted cluster version is loaded, then add coverage for orphaned keys on pre-25.4 and 25.4+ stores, including the mixed-version roachtest context described in the issue. Done means legacy stores warn and start while newer stores still fail the invariant.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
databases, distributed-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.