feat: native chunk query API for allocation-free result reading - #165
feat: native chunk query API for allocation-free result reading#165OzeroDev wants to merge 6 commits into
Conversation
3d14336 to
27c524f
Compare
runWithCtxInterrupt closed its done channel and awaited the interrupter only on the normal return path. In the case that fn panics, this would leave the goroutine running.
Split the string_t decoding of getBytes into getBytesView, which returns the bytes as they sit in the vector.
close freed the C-allocated chunk but left the vectors of the DataChunk pointing into that memory. Such behavior created a use-after-free bug that would result in process crashes instead of just failing the request. Also return early when a column fails to initialize, rather than reading the size of a chunk that was not fully set up.
The names a DataChunk already carries were only reachable through driver.Rows. Expose them so that code reading chunks directly can address columns by name.
GetVarcharView and GetBlobView return the bytes where DuckDB already holds them so that a caller can consume a result without allocating per row.
Reading a result through database/sql copies and boxes every value, which is significant overhead when the caller only forwards the rows. QueryChunksContext hands the native chunks to a callback instead, so values can be read straight out of the vectors.
| // Join the interrupter on every exit path, including a panic out of fn. | ||
| // An interrupter that outlives this call interrupts whatever runs on conn | ||
| // next, and dereferences conn even after the connection has been closed. | ||
| defer func() { |
|
This PR is currently marked as a draft as from our testing, we have found that the peak RSS grows in line with the size of the query result. Such behavior is not acceptable for our use case given the size of the objects that we work with, but this problem can be mitigated with streaming ( I have also taken a look at the scalar UDF chunk API, but it is invoked concurrently from all worker threads so chunks arrive out of order, and writing to the client from inside the callback blocks DuckDB's worker threads on consumer I/O. Furthermore, the scalar UDF chunk API would also require further work to reduce its GO heap allocation for allocation-free result reading. Given such circumstance, what does the best course of action look like for |
|
Hi @OzeroDev - I expect to discuss these changes and the path forward more with @EtgarDev tomorrow/this week, but I already wanted to get back to you on some of your points.
As you mentioned, there is also adjacent work here.
|
|
Thanks for working on this. We were very glad to see that this PR has arrived at a similar approach to the one we have been exploring independently for our downstream use case. We previously used duckdb-go through database/sql, but moved to duckdb-go-bindings directly and implemented our own connection pool and borrowed-chunk reader specifically to avoid the per-row memory copies and allocations in the row scanning path. We were very glad to see this PR arrive at a similar native-chunk approach while preserving the duckdb-go and database/sql integration. A supported API like this could allow us to move back to duckdb-go and remove a substantial amount of locally maintained bindings, pooling, cancellation, and result-lifetime code. Reducing memory copies is important for our large Parquet scans. Bounded RSS and true streaming are equally important, so we would be particularly interested in the C API V2 streaming version. We’d be happy to test it against our workload and share benchmark results if useful. |
Summary
Adds
QueryChunksContext, which executes a query and hands DuckDB's native data chunks to a callback, so a caller can read a result without the per-row copy allocations thatdatabase/sqlperforms.This extension is intended for use cases where the result is only forwarded rather than scanned into Go values (e.g. serialize to NDJSON/CSV and write it to a socket, a file, or another process). On that path every
rows.Scanallocation becomes a significant overhead as the value is copied out of the vector, boxed into adriver.Value, copied again into the destination, and then immediately discarded. Reading it throughdatabase/sqlallocated roughly two objects per row, so a large result turned into millions of short-lived allocations.There were two pre-existing
duckdb-gobugs found and addressed as they directly impacted the mechanics behind chunk query functionality.Both bug fixes are included in this PR as separate commits:
Join the context interrupter on panicrunWithCtxInterruptclosed its done channel and joined the interrupter goroutine only on the normal return path. Iffnpanicked, the goroutine survived the call and went on callingduckdb_interrupton a connectiondatabase/sqlhad already discarded and closed, faulting inside cgo. Found while testing a panicking callback.Invalidate data chunks on closeclosefreed the C chunk but left theDataChunkvectors pointing into that memory, so every getter and setter kept reading and writing it.Motivation
This feature extension came out of adding server-side SQL over stored objects to AIStore (NVIDIA's open-source, MIT-licensed storage stack for AI applications). Currently AIStore returns the full object a client requests but with a
duckdb-gointegration, aGETcan instead carry a SQL statement thatDuckDBcan 1. validate, 2. run against the locally stored object, and 3. stream the projected/filtered result back to the client.AIStore needs this path to be both efficient and reliable. This PR addresses both concerns: the chunk API removes the per-row allocations, and two pre-existing
duckdb-gobugs that the integration surfaced are fixed here as well. Nothing about the API itself is specific to this use case, it applies to any export or streaming path where rows are forwarded rather than inspected, and both bug fixes are independent of the new API entirely.Lifetime and ownership
The library owns every C allocation involved. The result is destroyed on return, each chunk is freed once
consumehas seen it, and the prepared statement is closed even ifconsumepanics.The views alias vector memory and are valid only until the chunk is released (the same behavior as
driver.Rows.Next, whose[]bytevalues are only valid until the next call). Because a caller can trivially retain the*DataChunkpast its callback, accessing a released chunk now returnserrClosedChunkrather than reading freed memory.API Usage
QueryChunksContext(ctx, *sql.Conn, query, consume, args...): invokes the consume function on every chunk until all query result chunks have been consumed. Returning an error fromconsumestops iteration and returns that error unchanged, so a sentinel can stop early.DataChunk.GetVarcharView/GetBlobView: the bytes where DuckDB already holds them.DataChunk.ColumnNames/ColumnName: the names of columns inDataChunk.database/sqlconverts them, so a query accepts the same types on both paths.See
chunk_query_bench_test.gofile for usage example.Benchmark
This PR also creates a
chunk_query_bench_test.gofile to benchmark the performance improvement that the native chunk query API provides over thedatabase/sqlusage.The benchmark serializes 100k rows to a single VARCHAR column and appends it into a reusable
buffer (
BenchmarkQueryChunksvsBenchmarkQueryRows):QueryChunksContext+ viewsdatabase/sqlrows +Scan~750x fewer allocations, ~580x less garbage, ~19% less wall time.
Commits
Each builds and passes tests on its own:
Join the context interrupter on panicExtract a borrowed-bytes vector getterInvalidate data chunks on closeAdd data chunk column name accessorsAdd borrowed views for VARCHAR and BLOB valuesAdd QueryChunksContextCommits 1 & 3 are duckdb-go fixes that affect chunk query implementation.
Commits 2, 4 & 5 provide helpers necessary for chunk query implementation.
Commit 6 introduces the core native chunk query API with full test suite and benchmark.
Test plan
TestQueryChunksContext(multi-chunk result, every row seen exactly once),TestQueryChunksContextMultipleStmts(only the last statement yields chunks),TestQueryChunksContextNoRows(consumenot called for an empty result),TestQueryChunksContextArgs(positional andsql.Named)TestQueryChunksContextViews(NULL vs. empty string, type mismatch, out-of-range row),TestQueryChunksContextInlineBoundary(lengths 1/11/12/13/24, i.e. both sides of thestring_tinline length where decoding switches from inlined bytes to an out-of-line pointer)TestQueryChunksContextColumnNamesTestQueryChunksContextAfterClose: a retained chunk returnserrClosedChunkfrom every getter and setter instead of reading freed memoryTestQueryChunksContextCancel(stops mid-stream, the connection stays usable afterwards),TestQueryChunksContextCanceledBeforeCall(consumenever runs)TestQueryChunksContextQueryError,TestQueryChunksContextConsumerError(sentinel returned unchanged),TestQueryChunksContextConsumerPanic,TestQueryChunksContextRejectsNilConsumer,TestQueryChunksContextRejectsNilConnection,TestRunWithCtxInterrupt_PanicJoinsInterrupter-tags=debug_bindingsthe bindings' allocation counters are zero after each of: happy path, consumer error mid-stream, query error, unsupported column type, panicking consumer-race, and under-tags=debug_bindings