Skip to content

Commit ecc333a

Browse files
committed
fix(duckdb): cap read_csv buffer to prevent OOM under low memory
DuckDB sizes its read_csv buffer as 16 × max_line_size and allocates it eagerly. The 256MB max_line_size raise (issue #787) thus demanded a 4 GiB block, causing OOM errors on hosts with a smaller memory_limit. Extract the read_csv from-expression into a new ReadCsvExpr helper that sets an explicit buffer_size equal to max_line_size, capping the allocation at the minimum needed to fit one line. Also add support for the memory_limit DuckDB session property, plus a regression unit test and replication config exercising a 5MB line under a 1GB memory limit.
1 parent ba205b0 commit ecc333a

4 files changed

Lines changed: 109 additions & 3 deletions

File tree

core/dbio/iop/duckdb.go

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,10 @@ func (duck *DuckDb) getSessionSettingsSQL() (sql string) {
463463
}
464464

465465
sql += fmt.Sprintf("SET http_timeout = %d;", httpTimeout)
466+
467+
if limit := duck.GetProp("memory_limit"); limit != "" {
468+
sql += fmt.Sprintf("SET memory_limit = '%s';", limit)
469+
}
466470
return
467471
}
468472

@@ -1934,10 +1938,8 @@ func (duck *DuckDb) DataflowToHttpStream(df *Dataflow, sc StreamConfig) (streamP
19341938
// Create a pipe to stream data through
19351939
pipeR, pipeW := io.Pipe()
19361940

1937-
maxLineSize := duck.MaxLineSize(batchR.Columns)
1938-
19391941
// can use this as a from table
1940-
fromExpr := g.F(`read_csv('%s', delim=',', header=True, columns=%s, max_line_size=%d, parallel=false, quote='"', escape='"', nullstr='\N', auto_detect=false)`, httpURL, duck.GenerateCsvColumns(batchR.Columns), maxLineSize)
1942+
fromExpr := duck.ReadCsvExpr(httpURL, batchR.Columns)
19411943

19421944
select {
19431945
case streamPartChn <- HttpStreamPart{
@@ -2029,6 +2031,16 @@ func (duck *DuckDb) MaxLineSize(columns Columns) int {
20292031
return DuckDbDefaultMaxLineSize
20302032
}
20312033

2034+
// ReadCsvExpr returns the read_csv from-expression for the CSV bridge.
2035+
// DuckDB sizes its read buffer as 16 × max_line_size and allocates it
2036+
// eagerly, so the 256MB raise would demand a 4 GiB block and OOM hosts
2037+
// with a smaller memory_limit. An explicit buffer_size caps the
2038+
// allocation at max_line_size, the minimum that fits one line.
2039+
func (duck *DuckDb) ReadCsvExpr(uri string, columns Columns) string {
2040+
maxLineSize := duck.MaxLineSize(columns)
2041+
return g.F(`read_csv('%s', delim=',', header=True, columns=%s, max_line_size=%d, buffer_size=%d, parallel=false, quote='"', escape='"', nullstr='\N', auto_detect=false)`, uri, duck.GenerateCsvColumns(columns), maxLineSize, maxLineSize)
2042+
}
2043+
20322044
func (duck *DuckDb) GenerateCsvColumns(columns Columns) (colStr string) {
20332045
// {'FlightDate': 'DATE', 'UniqueCarrier': 'VARCHAR', 'OriginCityName': 'VARCHAR', 'DestCityName': 'VARCHAR'}
20342046

core/dbio/iop/duckdb_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@ package iop
33
import (
44
"context"
55
"os"
6+
"strings"
67
"testing"
78
"time"
89

10+
"github.com/flarco/g"
911
"github.com/slingdata-io/sling-cli/core/dbio"
1012
"github.com/spf13/cast"
1113
"github.com/stretchr/testify/assert"
@@ -636,6 +638,49 @@ func TestDuckDbMaxLineSize(t *testing.T) {
636638
})
637639
}
638640

641+
// regression guard for the v1.5.25 OOM: DuckDB sizes its read_csv buffer as
642+
// 16 × max_line_size and allocates it eagerly. The 256MB raise thus demands a
643+
// 4 GiB block, which fails on hosts with memory_limit below ~4 GiB. The bridge
644+
// expression must cap the buffer so it works under small memory limits.
645+
func TestDuckDbReadCsvExprLowMemory(t *testing.T) {
646+
cols := NewColumnsFromFields("id", "payload")
647+
cols[0].Type = IntegerType
648+
cols[1].Type = TextType
649+
650+
// a ~5MB line exceeds the 2 MB default limit, so this also guards the
651+
// #787 raise: the line must still load with the capped buffer_size
652+
csvPath := os.TempDir() + "/duckdb_low_mem_test.csv"
653+
payload := strings.Repeat("x", 5*1024*1024)
654+
err := os.WriteFile(csvPath, []byte("id,payload\n1,"+payload+"\n"), 0644)
655+
if !assert.NoError(t, err) {
656+
return
657+
}
658+
defer os.Remove(csvPath)
659+
660+
duck := NewDuckDb(context.Background(), "memory_limit=1GB")
661+
err = duck.Open()
662+
if !assert.NoError(t, err) {
663+
return
664+
}
665+
defer duck.Close()
666+
667+
expr := duck.ReadCsvExpr(csvPath, cols)
668+
assert.Contains(t, expr, cast.ToString(DuckDbLargeMaxLineSize))
669+
670+
// same COPY shape the fabric/parquet bridge submits in production
671+
parquetPath := os.TempDir() + "/duckdb_low_mem_test.parquet"
672+
defer os.Remove(parquetPath)
673+
_, err = duck.Exec(g.F("COPY (select * from %s) TO '%s' (format 'parquet', overwrite true)", expr, parquetPath))
674+
if !assert.NoError(t, err) {
675+
return
676+
}
677+
678+
data, err := duck.Query(g.F("select count(*) cnt from read_parquet('%s')", parquetPath))
679+
if assert.NoError(t, err) && assert.Equal(t, 1, len(data.Rows)) {
680+
assert.EqualValues(t, 1, cast.ToInt(data.Rows[0][0]))
681+
}
682+
}
683+
639684
// regression guard for issue #770: http_timeout must be raised on every DuckDB
640685
// session, not only when an S3/httpfs secret registers the extension.
641686
func TestDuckDbHttpTimeout(t *testing.T) {
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# DuckDB sizes its read_csv buffer as 16 x max_line_size and allocates it
2+
# eagerly. The 256MB max_line_size raise must not demand a 4 GiB block when
3+
# memory_limit is small; the bridge caps the buffer with buffer_size.
4+
# One row holds a 5MB payload, which also guards the #787 large-line raise.
5+
source: LOCAL
6+
target: DUCK_LOW_MEM
7+
8+
defaults:
9+
mode: full-refresh
10+
11+
hooks:
12+
end:
13+
- type: check
14+
check: execution.status.error == 0
15+
on_failure: break
16+
17+
- type: query
18+
connection: '{target.name}'
19+
query: select count(*) cnt, max(length(payload)) max_len from main.big_rows
20+
into: result
21+
22+
- type: check
23+
check: int_parse(store.result[0].cnt) == 2
24+
25+
- type: check
26+
check: int_parse(store.result[0].max_len) == 5242880
27+
28+
- type: log
29+
message: 'SUCCESS: 5MB row loaded into duckdb with 1GB memory_limit'
30+
31+
streams:
32+
"file://temp/duckdb_low_mem/big.csv":
33+
object: main.big_rows

tests/suite.cli.yaml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2676,3 +2676,19 @@
26762676
sling run -d --src-stream 'http://127.0.0.1:18937/' --tgt-object 'file://temp/merge_latency/out.csv'
26772677
output_contains:
26782678
- 'execution succeeded'
2679+
2680+
# DuckDB allocates its read_csv buffer as 16 x max_line_size, so the 256MB
2681+
# raise (#787) demanded a 4 GiB block and OOMed hosts with memory_limit
2682+
# below ~4 GiB. The bridge must cap the buffer via buffer_size.
2683+
# Unit guard: TestDuckDbReadCsvExprLowMemory in core/dbio/iop/duckdb_test.go
2684+
- id: 321
2685+
name: 'duckdb csv bridge loads large rows under low memory_limit'
2686+
env:
2687+
DUCK_LOW_MEM: '{type: duckdb, instance: "temp/duckdb_low_mem/test.duckdb", memory_limit: 1GB}'
2688+
run: |
2689+
mkdir -p temp/duckdb_low_mem
2690+
rm -f temp/duckdb_low_mem/test.duckdb temp/duckdb_low_mem/big.csv
2691+
python3 -c 'import csv; w=csv.writer(open("temp/duckdb_low_mem/big.csv","w")); w.writerow(["id","payload"]); w.writerow([1,"x"*5242880]); w.writerow([2,"small"])'
2692+
sling run -d -r tests/replications/r.103.duckdb_low_mem_buffer.yaml
2693+
output_contains:
2694+
- 'SUCCESS: 5MB row loaded into duckdb with 1GB memory_limit'

0 commit comments

Comments
 (0)