cockroachdb / cockroachdb/cockroach
util/span: frontier entry is returned to the pool before it is unlinked
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Summary:**
`span.btreeFrontier.deleteEntry` registers `defer putFrontierEntry(e)` *before* removing `e` from the heap and the tree. If anything between the defer and `f.tree.Delete(e)` fails — an `expensiveChecksEnabled()` error return, or a panic in `heap.Remove` on a stale `heapIdx` — the entry is returned to the package-global pool while still linked into the tree. `btreeFrontier.Release` then walks the tree and pools every entry it finds, returning the same pointer a second time.
Two unrelated frontiers then alias one entry. A frontier gates emitted resolved timestamps, so the observable failure is a resolved timestamp ahead of unemitted data, for changefeeds, PCR and LDR. There is no error, no log and no metric on this path.
The error-return case does not require a panic at all and reproduces in `crdb_test` builds today. Part of #174944.
**Findings:**
- `span.btreeFrontier.deleteEntry`: `e` (`*btreeFrontierEntry`) is pooled before it is unlinked, and `btreeFrontier.Release` pools it again.
- No item pool in the package has a double-`Put` guard, which is why this class is undetectable in the field: a reissued pooled object surfaces only as wrong answers, arbitrarily later, in unrelated work.
**Next Steps:**
- [ ] Move `putFrontierEntry(e)` after `f.tree.Delete(e)` and after the `checkSpan` error return
- [ ] Guard `heap.Remove` on `e.heapIdx >= 0`
- [ ] Add a `pooled bool` double-`Put` assertion to `btreeFrontierEntry` under `crdb_test`
Epic: none
Jira issue: CRDB-68119
Contributor guide
Research direction
Start at span.btreeFrontier.deleteEntry and btreeFrontier.Release, tracing the ordering around checkSpan, heap.Remove, f.tree.Delete(e), and putFrontierEntry(e). Reproduce the error-return path in a crdb_test build and exercise stale heapIdx handling. Done means entries are unlinked before pooling and the pooled bool assertion catches any double Put.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- databases, distributed-systems
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100