Skip to content

Commit 458ee2a

Browse files
committed
feat(resources #71): cgroup-aware resource-limit observability + regression guard
Issue #71 proposed an opt-in --auto-memory flag on the premise that DuckDB sizes max_memory from host RAM and ignores container cgroup limits, leading to kernel OOM kills (exit 137). Empirical testing of the bundled DuckDB 1.5.2 shows that premise is obsolete: DuckDB already honors cgroup limits, sizing max_memory to ~80% of the cgroup memory limit and threads from the CPU quota. A ~3 GiB query in a 1 GiB container fails with a graceful "Out of Memory" error and the server survives -- no kernel kill. Rather than ship a redundant override, this adds observability and a guard: - src/system_resources.{hpp,cpp}: cgroup v1/v2 memory + CPU detection with injectable /sys/fs/cgroup roots; unit-tested in system_resources_test.cpp. - database_manager: log a startup "resource-limits:" line reporting the detected ceiling and source (cgroup-v2 / cgroup-v1 / meminfo / hardware), so the limit flApi runs under is visible. No DuckDB setting is changed; duckdb.max_memory / duckdb.threads still override when set explicitly. - test/integration/smoke_resource_limits.sh + `make smoke-test-resources`: Docker regression guard asserting graceful degradation (HTTP 500 OOM, server survives) and the cgroup-detection startup log. Skips off Linux/Docker. - docs: DESIGN_DECISIONS §10 records the finding; CONFIG_REFERENCE notes the cgroup behavior and how to pin explicit values.
1 parent eae15c1 commit 458ee2a

16 files changed

Lines changed: 762 additions & 5 deletions

CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,7 @@ add_library(flapi-lib STATIC
272272
src/sql_parameter_classifier.cpp
273273
src/sql_template_processor.cpp
274274
src/sql_utils.cpp
275+
src/system_resources.cpp
275276
src/mcp_server.cpp
276277
src/mcp_tool_handler.cpp
277278
src/mcp_tool_rate_limiter.cpp

Makefile

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# Makefile for flAPI project
22

33
# Phony targets
4-
.PHONY: all debug release clean run-debug run-release run-integration-tests docker-build web cli-build cli-test vscode-build vscode-dev integration-test integration-tests integration-test-rest integration-test-mcp integration-test-ducklake integration-test-examples integration-test-setup integration-test-ci test-all help
4+
.PHONY: all debug release clean run-debug run-release run-integration-tests docker-build web cli-build cli-test vscode-build vscode-dev integration-test integration-tests integration-test-rest integration-test-mcp integration-test-ducklake integration-test-examples integration-test-setup integration-test-ci smoke-test-resources test-all help
55

66
# Check if Ninja is available
77
NINJA := $(shell which ninja)
@@ -309,6 +309,13 @@ integration-test-ci: release integration-test-setup
309309
echo "Integration tests completed with exit code: $$TEST_RESULT"; \
310310
exit $$TEST_RESULT
311311

312+
# Container smoke test for cgroup-aware memory sizing (#71).
313+
# Runs the release binary in a memory-limited Docker container and asserts
314+
# DuckDB honors the cgroup limit (graceful OOM, server survives). Skips on
315+
# non-Linux / no-Docker hosts. See test/integration/smoke_resource_limits.sh.
316+
smoke-test-resources: release
317+
@bash test/integration/smoke_resource_limits.sh
318+
312319
# Build Docker image
313320
docker: release
314321
@echo "Building Docker image..."

docs/CONFIG_REFERENCE.md

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -300,8 +300,16 @@ duckdb:
300300
**Notes:**
301301
- Omit `db_path` or set to `:memory:` for an in-memory database
302302
- Any key-value pairs are passed directly as DuckDB settings
303-
304-
> **Implementation:** `src/database_manager.cpp` | **Tests:** `test/cpp/database_manager_test.cpp`
303+
- **Containers (cgroup limits):** when `threads`/`max_memory` are omitted (the
304+
default), DuckDB sizes them from the container's cgroup limits — `max_memory`
305+
to ~80% of the cgroup memory limit and `threads` from the CPU quota (Kubernetes
306+
limits, Docker `--memory`/`--cpus`, etc.). You normally do **not** need to set
307+
these per deployment target. To pin explicit values, set them here and they are
308+
passed straight through. flAPI logs the detected ceiling at startup as
309+
`resource-limits: memory source=cgroup-v2 available=...MB` / `cpu source=...`,
310+
so the limit it is actually running under is visible in the logs (#71).
311+
312+
> **Implementation:** `src/database_manager.cpp`, `src/system_resources.cpp` | **Tests:** `test/cpp/database_manager_test.cpp`, `test/cpp/system_resources_test.cpp`
305313

306314
### 2.5 DuckLake Configuration
307315

docs/spec/DESIGN_DECISIONS.md

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,47 @@ four supported platforms (#49 / `.github/workflows/build.yaml`).
416416

417417
---
418418

419+
## 10. Container memory/CPU limits: rely on DuckDB's cgroup awareness (#71)
420+
421+
**Decision:** Do **not** add a flApi-side mechanism to size `max_memory` /
422+
`threads` from container cgroup limits. Instead, rely on DuckDB's own
423+
cgroup-aware detection and simply **log the detected limits at startup** for
424+
observability.
425+
426+
**Context:** Issue #71 proposed an opt-in `--auto-memory` flag, on the premise
427+
that DuckDB reads *host* RAM (via `/proc/meminfo`, `sysconf`) and ignores
428+
container cgroup limits — so an unbounded buffer pool inside a small container
429+
would be kernel-OOM-killed (exit 137) rather than failing gracefully.
430+
431+
**Finding (empirically verified, DuckDB 1.5.2):** that premise no longer holds.
432+
Running the binary in memory/CPU-limited containers shows DuckDB **already**
433+
honors cgroup limits:
434+
435+
| Container limit | DuckDB `memory_limit` (no config) | DuckDB `threads` (no config) |
436+
|---|---|---|
437+
| `--memory=512m` | 409.5 MiB (≈80% of cgroup) | — |
438+
| `--memory=1g --cpus=2` | 819 MiB (≈80% of cgroup) | 2 (from CPU quota) |
439+
440+
A ~3 GiB query in a 1 GiB container raises a **graceful** DuckDB "Out of Memory"
441+
error and the server survives — no kernel kill. (The `using 32 threads` startup
442+
line is Crow's HTTP thread pool, not DuckDB query parallelism — a red herring.)
443+
444+
**Consequences:**
445+
- A flApi-side override would, at best, duplicate DuckDB's behavior; its only
446+
unique value would be a tunable percentage (DuckDB hardcodes ~80%). That did
447+
not justify the cross-platform detection/precedence surface area, so it was
448+
dropped.
449+
- `src/system_resources.{hpp,cpp}` provides cgroup v1/v2 memory + CPU detection
450+
(unit-tested with injectable `/sys/fs/cgroup` roots) used **only** to emit a
451+
startup `resource-limits:` INFO log, so the ceiling flApi runs under is
452+
visible and container misconfiguration is diagnosable.
453+
- **To override** the defaults, set `duckdb.max_memory` / `duckdb.threads`
454+
explicitly in `flapi.yaml` (these are passed straight through to DuckDB).
455+
- `test/integration/smoke_resource_limits.sh` (`make smoke-test-resources`) is a
456+
Docker regression guard asserting the graceful-degradation behavior.
457+
458+
---
459+
419460
## Summary
420461

421462
These design decisions prioritize:

src/database_manager.cpp

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "sql_utils.hpp"
1313
#include "database_manager.hpp"
1414
#include "duckdb_raii.hpp"
15+
#include "system_resources.hpp"
1516

1617
namespace flapi {
1718

@@ -259,7 +260,16 @@ void DatabaseManager::createAndInitializeDuckDBConfig(std::shared_ptr<ConfigMana
259260
throw std::runtime_error("Failed to set DuckDB configuration: autoload_known_extensions");
260261
}
261262

262-
// Apply settings from the configuration
263+
// Observability (#71): log the memory / CPU limits we detect for this
264+
// process (honoring container cgroup limits). This does NOT change any
265+
// DuckDB setting -- DuckDB 1.5.2 is itself cgroup-aware and sizes
266+
// max_memory (~80% of the limit) and threads from the same limits. The
267+
// log makes container misconfiguration diagnosable and lets operators see
268+
// the ceiling flAPI is actually running under. To override, set
269+
// duckdb.max_memory / duckdb.threads explicitly in flapi.yaml.
270+
logDetectedResourceLimits(config_manager);
271+
272+
// Apply settings from the configuration.
263273
const auto& duckdb_settings = config_manager->getDuckDBConfig().settings;
264274
for (const auto& [key, value] : duckdb_settings) {
265275
if (duckdb_set_config(config, key.c_str(), value.c_str()) == DuckDBError) {
@@ -269,6 +279,34 @@ void DatabaseManager::createAndInitializeDuckDBConfig(std::shared_ptr<ConfigMana
269279
}
270280
}
271281

282+
void DatabaseManager::logDetectedResourceLimits(std::shared_ptr<ConfigManager> config_manager) {
283+
const auto& settings = config_manager->getDuckDBConfig().settings;
284+
285+
flapi::MemoryDetection mem = flapi::DetectAvailableMemoryBytes();
286+
if (mem.source != "unknown" && mem.bytes > 0) {
287+
CROW_LOG_INFO << "resource-limits: memory source=" << mem.source
288+
<< " available=" << (mem.bytes / (1024ull * 1024ull)) << "MB"
289+
<< (settings.count("max_memory") > 0
290+
? " (overridden by duckdb.max_memory=" + settings.at("max_memory") + ")"
291+
: " (DuckDB sizes max_memory from this limit)");
292+
} else {
293+
CROW_LOG_DEBUG << "resource-limits: could not detect a memory limit; "
294+
<< "DuckDB will use its own detection.";
295+
}
296+
297+
flapi::CpuDetection cpu = flapi::DetectAvailableCpuCount();
298+
if (cpu.source != "unknown" && cpu.cores >= 1) {
299+
CROW_LOG_INFO << "resource-limits: cpu source=" << cpu.source
300+
<< " cores=" << cpu.cores
301+
<< (settings.count("threads") > 0
302+
? " (overridden by duckdb.threads=" + settings.at("threads") + ")"
303+
: " (DuckDB sizes threads from this limit)");
304+
} else {
305+
CROW_LOG_DEBUG << "resource-limits: could not detect a CPU quota; "
306+
<< "DuckDB will use its own detection.";
307+
}
308+
}
309+
272310
void DatabaseManager::loadDefaultExtensions(std::shared_ptr<ConfigManager> config_manager) {
273311
auto extensions = config_manager->getDuckDBConfig().default_extensions;
274312

src/include/database_manager.hpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,12 @@ class DatabaseManager : public std::enable_shared_from_this<DatabaseManager> {
100100
void logDuckDBVersion();
101101

102102
void createAndInitializeDuckDBConfig(std::shared_ptr<ConfigManager> config_manager, duckdb_config& config);
103-
103+
104+
// Observability (#71): log the cgroup-detected memory / CPU limits at
105+
// startup. Does not change any DuckDB setting (DuckDB is itself
106+
// cgroup-aware); purely diagnostic so the running ceiling is visible.
107+
void logDetectedResourceLimits(std::shared_ptr<ConfigManager> config_manager);
108+
104109

105110
std::string processTemplate(const EndpointConfig& endpoint, std::map<std::string, std::string>& params);
106111
std::string processCacheTemplate(const EndpointConfig& endpoint, const CacheConfig& cacheConfig, std::map<std::string, std::string>& params);

src/include/system_resources.hpp

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
#pragma once
2+
3+
#include <cstdint>
4+
#include <optional>
5+
#include <string>
6+
7+
namespace flapi {
8+
9+
// Result of probing the memory available to this process. `bytes` is the
10+
// detected ceiling (cgroup limit or host physical memory, whichever is
11+
// smaller); `source` names where the value came from so it can be logged.
12+
// On failure `bytes == 0` and `source == "unknown"`.
13+
struct MemoryDetection {
14+
uint64_t bytes = 0;
15+
std::string source = "unknown"; // cgroup-v2|cgroup-v1|meminfo|sysconf|sysctl|win-global|unknown
16+
};
17+
18+
// Result of probing the CPU quota available to this process. `cores` is
19+
// clamped to at least 1 when a positive value is detected.
20+
struct CpuDetection {
21+
uint32_t cores = 0;
22+
std::string source = "unknown"; // cgroup-v2|cgroup-v1|hardware|unknown
23+
};
24+
25+
// --- Pure parsers (no filesystem; unit-tested directly) ------------------
26+
27+
// cgroup v2 `memory.max`. A literal "max" means unlimited -> nullopt.
28+
std::optional<uint64_t> ParseCgroupV2MemoryMax(const std::string& content);
29+
30+
// cgroup v1 `memory.limit_in_bytes`. Absurdly large sentinel values
31+
// (PAGE_COUNTER_MAX, 0x7FFFFFFFFFFFF000, etc.) mean unlimited -> nullopt.
32+
std::optional<uint64_t> ParseCgroupV1MemoryLimit(const std::string& content);
33+
34+
// cgroup v2 `cpu.max` == "<quota> <period>". "max ..." means unlimited ->
35+
// nullopt. Otherwise effective cores = ceil(quota / period), clamped >= 1.
36+
std::optional<uint32_t> ParseCgroupV2CpuMax(const std::string& content);
37+
38+
// cgroup v1 `cpu.cfs_quota_us` / `cpu.cfs_period_us`. quota -1 means
39+
// unlimited -> nullopt. Otherwise ceil(quota / period), clamped >= 1.
40+
std::optional<uint32_t> ParseCgroupV1Cpu(const std::string& quota, const std::string& period);
41+
42+
// --- Composing detectors (probe filesystem; `root` is injectable for tests) ---
43+
44+
MemoryDetection DetectAvailableMemoryBytes(const std::string& root = "/");
45+
CpuDetection DetectAvailableCpuCount(const std::string& root = "/");
46+
47+
} // namespace flapi

0 commit comments

Comments
 (0)