cockroachdb / cockroachdb/cockroach
sql: abandoned internal executor iterators wedge a connExecutor goroutine and block node drain
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Summary:**
An internal-executor iterator that is abandoned rather than drained wedges its `connExecutor` goroutine permanently, and one of the things it holds is a stopper task — so a single abandoned iterator can prevent the node from draining. Part of #174944.
Exhaustion auto-closes the iterator, which is why these sites usually work; they fail on the abnormal exit — a panic, an early `return` inside the loop body, or a `continue` that re-opens the iterator.
**Findings:**
- `sql.rowsIterator` / `sql.StmtBuf.CurCmd`: an abandoned iterator (one whose `Next` last returned true) never closes the `StmtBuf`, and the `connExecutor` goroutine then sits in `buf.mu.cond.Wait()` with no context and no `ShouldQuiesce`. Per abandoned iterator: one wedged goroutine, one never-released stopper task so `Stopper.Stop` and node drain hang, a permanent `SessionRegistry` entry walked by every query-cancel lookup, and an unfinished span.
- `QueryIterator` / `QueryIteratorEx` call sites without a deferred `Close`: 27 of 123 acquire-and-consume sites (22 with no close at all, 5 closing inline only). Each is one instance of the above.
- `sql.internalDB` / `schemaChangerState.memAcc`: the bound account is constructed at one point and cleared only in the commit path's defer, so an error or panic before that point leaves it never cleared.
**Next Steps:**
- [ ] Select on `ShouldQuiesce` in `StmtBuf.CurCmd`, or have `InternalDB` track open `StmtBuf`s and close them on drain — this turns "node can never drain" into "node drains"
- [ ] Add `defer func() { err = errors.CombineErrors(err, it.Close()) }()` immediately after each of the 27 `QueryIterator` acquisitions
- [ ] Defer the `memAcc` clear at construction
Epic: none
Jira issue: CRDB-68122
Epic CRDB-65516
Contributor guide
Research direction
Locate sql.rowsIterator and sql.StmtBuf.CurCmd first, then trace the QueryIterator and QueryIteratorEx acquisition sites listed in the issue. Review sql.internalDB and schemaChangerState.memAcc for the account cleanup path. Done means abandoned iterators no longer block node draining, all 27 acquisitions defer Close, and memAcc is cleared on error or panic.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, sql
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100