Closing a keyless index scan can leave its producer blocked
perfloop-oss/dolt · RESOURCE LEAK
https://perfloop.ai/t/oss/case_rh9qgdtpp8
Verdict
VALIDATED BY A DIFFERENT CHANGE · settled 2026-10-10 · merged as dolthub/dolt#12066
What happened: The assertion is violated on the comparison and satisfied with this change.
Hypothesis
An early-closed keyless secondary-index scan can leave its row-producing goroutine blocked on its output channel. If the caller keeps the constructor's context alive and stops reading while more than eight rows remain to send, Close returns without stopping that worker. The worker can keep the scan state and its current expanded duplicate group reachable. Production occurrence and retention duration are unmeasured; this proposal does not claim a CPU or latency gain.
The scope is successfully constructed prolly-format keyless secondary-index scans at revision 94cc6dd5a46457fb69e9e33a6a949ab369b3c82b, reached through IndexedDoltTable.PartitionRows. NewIndexReaderBuilder selects keylessIndexImplBuilder when sql.IsKeyless is true, before considering covering lookups (index_reader.go:301-308). Its NewPartitionRowIter calls this constructor with the selected range, projections, primary and secondary maps, and scan direction (768-779). The constructor creates an errgroup child context, an eight-row channel, and a worker, but retains no cancellation function (prolly_index_iter.go:420-438). queueRows expands each duplicate group and then waits on either that context or a channel send (457-497). prollyKeylessIndexIter.Close only returns nil (519-521). Next waits for the worker only after the channel has closed (444-455).
The property to repair is that, with no Next call in flight, the iterator has no owned producer still running when Close returns, without cancelling its caller's shared context. Ten successfully decoded identical matching rows, one consumed row, and an otherwise live context suffice to exercise a blocked sender: nine rows remain, exceeding the channel's capacity of eight. This condition does not apply to every invocation. Full consumption, upstream termination, or parent-context cancellation can let the worker exit. In particular, the go-mysql-server version pinned at go/go.mod:68 cancels the server query context on return from server.Handler.doQuery, and whole-query tracking also performs EndQuery cleanup. No permanent accumulation across completed server requests is asserted. Its LimitIter stops pulling at the limit and its Close delegates to the child; TableRowIter passes the incoming context to PartitionRows rather than creating a scan-owned cancellation context. Thus an individual scan's lifetime is not established by whole-query cleanup.
The Case should add a deterministic early-close regression using valid primary and secondary prolly maps for a keyless indexed table, then exercise the accepted PartitionRows route with a still-live caller context. Establish that the producer has reached the full-buffer send, consume one of ten matching rows, and close the scan without cancelling that context. The baseline signal is that producer completion remains pending after Close and completes only after explicit parent cancellation; completion at Close would refute the defect under these premises. The repaired scan must have no outstanding producer at Close. A SQL integration check should confirm the secondary-index binding with FORCE INDEX and LIMIT 1. Required result regressions cover full duplicate consumption, projected-column order, forward/reverse scans, independent row slices, and required EOF/error behavior. These are correctness controls, not evidence of a benefiting performance population.
This removes the iterator-owned asynchronous delivery lifetime as a class rather than relying on cleanup at a larger query boundary. The direction changes private execution state only; it adds no SQL operation, configuration, storage format, or protocol. README.md:5-9 and 60-61 document the SQL database and MySQL-compatible server consumers. Those passages do not establish a retirement decision or maintainer agreement for this iterator, and no captured maintenance decision is being assumed.
Change to test: Move row production into pull-owned iterator state instead of the background goroutine and channel, so closing a keyless secondary-index scan leaves no producer waiting for later parent-context cancellation. Keep the existing sql.RowIter contract, duplicate cardinality, projection order, scan direction, independent returned row slices, and required EOF/error behavior without adding a public API.
Where it lives
perfloop-oss/dolt · go/libraries/doltcore/sqle/indexed_dolt_table.go
Evidence
The assertion is violated on the comparison and satisfied with this change: `With no Next in flight, after one of ten matching duplicate rows is consumed from a successfully constructed prolly-format keyless secondary-index scan using the iterator constructor reached by IndexedDoltTable.PartitionRows, Close returns only when no iterator-owned producer remains running and the caller context remains live.`
Checks: 9 of 9 passed. Verification: no defect found.