diff --git a/internal/config/pinned_memory_test.go b/internal/config/pinned_memory_test.go new file mode 100644 index 00000000..74de5a80 --- /dev/null +++ b/internal/config/pinned_memory_test.go @@ -0,0 +1,67 @@ +package config + +import "testing" + +// Pinning storage.duckdb.memory skips detection entirely, so nothing compares +// the value to the machine. Both production deployments pinned 4GB on 8 GiB +// hosts and were OOM-killed repeatedly: DuckDB's budget is not the process's +// budget, and the Go runtime, the ingest appender and compaction's merges all +// sit on top of it. The operator keeps the last word -- this warns, it does not +// refuse -- but a value that cannot fit should not pass in silence. +func TestPinnedMemoryConcern(t *testing.T) { + const eightGB = 8 << 30 + tests := []struct { + name string + pinned string + detected uint64 + wantWarns bool + }{ + {name: "a pin inside the automatic share is fine", pinned: "4GB", detected: 16 << 30}, + {name: "the share the machine would have chosen is fine", pinned: "4800MB", detected: eightGB}, + {name: "half the machine is fine", pinned: "4GB", detected: eightGB}, + {name: "most of the machine leaves nothing for the runtime", pinned: "7GB", detected: eightGB, wantWarns: true}, + {name: "more than the machine is always wrong", pinned: "16GB", detected: eightGB, wantWarns: true}, + {name: "gibibytes parse too", pinned: "7GiB", detected: eightGB, wantWarns: true}, + {name: "nothing pinned, nothing to say", pinned: "", detected: eightGB}, + {name: "undetectable machine cannot be judged", pinned: "7GB", detected: 0}, + {name: "unparseable pin is not silently treated as zero", pinned: "lots", detected: eightGB, wantWarns: true}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := pinnedMemoryConcern(test.pinned, test.detected) + if gotWarns := got != ""; gotWarns != test.wantWarns { + t.Errorf("pinnedMemoryConcern(%q, %d) = %q; warning expected: %v", + test.pinned, test.detected, got, test.wantWarns) + } + }) + } +} + +func TestParseByteSize(t *testing.T) { + tests := []struct { + in string + want uint64 + ok bool + }{ + {in: "4GB", want: 4 << 30, ok: true}, + {in: "4GiB", want: 4 << 30, ok: true}, + {in: "512MB", want: 512 << 20, ok: true}, + {in: "8192", want: 8192, ok: true}, + {in: "6.4GiB", want: 6871947673, ok: true}, + {in: " 2 GB ", want: 2 << 30, ok: true}, + {in: "", ok: false}, + {in: "lots", ok: false}, + {in: "-4GB", ok: false}, + } + for _, test := range tests { + t.Run(test.in, func(t *testing.T) { + got, ok := parseByteSize(test.in) + if ok != test.ok { + t.Fatalf("parseByteSize(%q) ok = %v, want %v", test.in, ok, test.ok) + } + if ok && got != test.want { + t.Errorf("parseByteSize(%q) = %d, want %d", test.in, got, test.want) + } + }) + } +} diff --git a/internal/config/sizing.go b/internal/config/sizing.go index 22c78fc5..3065d7f4 100644 --- a/internal/config/sizing.go +++ b/internal/config/sizing.go @@ -142,6 +142,16 @@ func (c Config) logResolvedSizing(src sizingSource) { if src.MemoryAuto && c.DuckDBMemory == "" { slog.Warn("could not detect available memory; DuckDB will size itself to 80% of RAM, which can exceed the machine once the Go runtime is added — configure storage.duckdb.memory explicitly") } + // A pinned budget skipped detection, so this is the only place it is ever + // compared to the machine it will run on. + if !src.MemoryAuto { + if concern := pinnedMemoryConcern(c.DuckDBMemory, detectMemory().available); concern != "" { + slog.Warn("pinned DuckDB memory budget looks too large for this machine", + "concern", concern, + "duckdb_memory", c.DuckDBMemory, + ) + } + } } // resolveDuckDBMemory returns a DuckDB memory budget for a machine of the given @@ -161,6 +171,72 @@ func resolveDuckDBMemory(available uint64) string { return fmt.Sprintf("%dMB", budgetMB) } +// pinnedMemoryConcern reports why an operator-pinned DuckDB budget looks wrong +// for this machine, or "" when it looks fine or cannot be judged. +// +// Pinning storage.duckdb.memory skips detection entirely (resolveSizing only +// fills a value that is empty), so without this nothing ever compares the +// figure to the machine it will run on. DuckDB's budget is not the process's +// budget: the Go heap, the ingest appender and compaction's merges all sit on +// top of it, which is why duckDBMemoryPercent leaves 40% of the machine alone +// when it chooses for itself. A pin above that share is judged by the same +// standard. +// +// This warns rather than refuses. An operator who has measured their workload +// may legitimately know better than the default share, and failing startup on +// a configuration that has been running would trade a survivable problem for +// an outage. But a value that cannot fit should not pass in silence. +func pinnedMemoryConcern(pinned string, detectedRAM uint64) string { + if strings.TrimSpace(pinned) == "" || detectedRAM == 0 { + return "" + } + budget, ok := parseByteSize(pinned) + if !ok { + return fmt.Sprintf("storage.duckdb.memory %q could not be parsed as a size; DuckDB may reject it or fall back to its own default", pinned) + } + if budget >= detectedRAM { + return fmt.Sprintf("storage.duckdb.memory %q is at or above the %d bytes detected for this machine; the Go runtime and compaction allocate on top of it", pinned, detectedRAM) + } + share := detectedRAM / 100 * duckDBMemoryPercent + if budget > share { + return fmt.Sprintf("storage.duckdb.memory %q takes more than the %d%% of %d bytes automatic sizing would leave DuckDB; the remainder has to cover the Go heap, the ingest appender and compaction merges", pinned, duckDBMemoryPercent, detectedRAM) + } + return "" +} + +// parseByteSize parses the size forms an operator is likely to write: a bare +// byte count, or a decimal with a GB/GiB/MB/MiB/KB/KiB suffix. DuckDB treats +// the decimal and binary suffixes alike, so this does too. +func parseByteSize(value string) (uint64, bool) { + text := strings.TrimSpace(value) + if text == "" { + return 0, false + } + units := []struct { + suffix string + scale float64 + }{ + {"GIB", 1 << 30}, {"GB", 1 << 30}, + {"MIB", 1 << 20}, {"MB", 1 << 20}, + {"KIB", 1 << 10}, {"KB", 1 << 10}, + {"B", 1}, + } + upper := strings.ToUpper(text) + scale := float64(1) + for _, unit := range units { + if strings.HasSuffix(upper, unit.suffix) { + scale = unit.scale + text = strings.TrimSpace(text[:len(text)-len(unit.suffix)]) + break + } + } + amount, err := strconv.ParseFloat(strings.TrimSpace(text), 64) + if err != nil || amount < 0 { + return 0, false + } + return uint64(amount * scale), true +} + // resolveDuckDBMaxConns sizes the connection pool from available parallelism. func resolveDuckDBMaxConns(cores int) int { conns := cores