feat: make LMDB max_dbs and map_size configurable by Harry-kp · Pull Request #1704 · cocoindex-io/cocoindex

Harry-kp · 2026-02-24T20:26:17Z

Summary

Exposes the hardcoded LMDB max_dbs (1024) and map_size (4 GiB) as configurable settings
Configurable via Python Settings(lmdb_max_dbs=..., lmdb_map_size=...) or environment variables COCOINDEX_LMDB_MAX_DBS / COCOINDEX_LMDB_MAP_SIZE
Defaults are set on the Python side (matching the source_max_inflight_rows pattern), so fields are always concrete values
Invalid values are validated on the Rust side with client_bail!

Fixes #1703

Changes

rust/core/src/engine/environment.rs — Added lmdb_max_dbs: u32 and lmdb_map_size: usize to EnvironmentSettings with #[serde(default = "...")] for backward compatibility. Validates max_dbs >= 1 and map_size > 0 via client_bail!. Removed unused u32 import and the // TODO: Expose these as settings comment.
python/cocoindex/_internal/setting.py — Added lmdb_max_dbs: int = 1024 and lmdb_map_size: int = 0x1_0000_0000 to Settings, loaded from env vars in from_env()
python/cocoindex/setting.py — Same changes (public API mirror, noted as deprecated in v1)

Usage

import cocoindex

settings = cocoindex.Settings(
    db_path="/tmp/cocoindex_db",
    lmdb_max_dbs=2048,
    lmdb_map_size=0x2_0000_0000,  # 8 GiB
)

Or via environment variables:

export COCOINDEX_LMDB_MAX_DBS=2048
export COCOINDEX_LMDB_MAP_SIZE=8589934592

Expose the previously hardcoded LMDB `max_dbs` (1024) and `map_size` (4 GiB) as configurable settings, both programmatically and via environment variables `COCOINDEX_LMDB_MAX_DBS` / `COCOINDEX_LMDB_MAP_SIZE`. When not set, the existing defaults are preserved. Fixes cocoindex-io#1703 Signed-off-by: Harry-kp <chaudharyharshit9@gmail.com>

Copilot

Pull request overview

This PR makes LMDB max_dbs and map_size configurable (previously hardcoded in the Rust core), exposing them through Python Settings(...) and environment variables COCOINDEX_LMDB_MAX_DBS / COCOINDEX_LMDB_MAP_SIZE while preserving existing defaults when unset.

Changes:

Add optional lmdb_max_dbs / lmdb_map_size to Rust EnvironmentSettings and apply them when opening the LMDB environment (fallback to prior defaults).
Add lmdb_max_dbs / lmdb_map_size to Python Settings (public + internal mirror) and load them from env vars in Settings.from_env().

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File	Description
rust/core/src/engine/environment.rs	Adds configurable LMDB settings and uses them when opening `heed::Env`.
python/cocoindex/setting.py	Exposes LMDB settings in public `Settings` and loads them from env vars.
python/cocoindex/_internal/setting.py	Mirrors the public settings/env loading for internal API surface.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-24T20:32:03Z

python/cocoindex/setting.py

+        lmdb_kwargs: dict[str, Any] = dict()
+        _load_field(
+            lmdb_kwargs,
+            "lmdb_max_dbs",
+            "COCOINDEX_LMDB_MAX_DBS",
+            parse=int,
+        )
+        _load_field(
+            lmdb_kwargs,
+            "lmdb_map_size",
+            "COCOINDEX_LMDB_MAP_SIZE",
+            parse=int,
+        )


New environment variables COCOINDEX_LMDB_MAX_DBS / COCOINDEX_LMDB_MAP_SIZE are introduced here, but there doesn’t appear to be test coverage asserting they’re honored by Settings.from_env(). Please add a unit/integration test that sets these env vars and verifies the resulting Settings carries the parsed values.

@georgeh0 The existing GlobalExecutionOptions.source_max_inflight_rows (the pattern we're following) doesn't have dedicated unit tests for env var loading either. Would you like me to add test coverage for the LMDB env vars, or is the Rust-side validation sufficient for now?

It's good for now. Thanks!

Copilot · 2026-02-24T20:32:04Z

rust/core/src/engine/environment.rs

@@ -5,13 +5,16 @@ use crate::{
 use serde::{Deserialize, Serialize};
 use std::{collections::BTreeSet, path::PathBuf, u32};


u32 is imported from std but no longer used in this module. Please remove it to avoid unused-import warnings (and potential CI failures if warnings are denied).

Suggested change

use std::{collections::BTreeSet, path::PathBuf, u32};

use std::{collections::BTreeSet, path::PathBuf};

Fixed in f2308d6.

Copilot · 2026-02-24T20:32:04Z

rust/core/src/engine/environment.rs

+        let max_dbs = settings.lmdb_max_dbs.unwrap_or(DEFAULT_MAX_DBS);
+        let map_size = settings.lmdb_map_size.unwrap_or(DEFAULT_LMDB_MAP_SIZE);
        let db_env = unsafe {
            heed::EnvOpenOptions::new()
-                .max_dbs(MAX_DBS)
-                .map_size(LMDB_MAP_SIZE)
+                .max_dbs(max_dbs)
+                .map_size(map_size)


lmdb_max_dbs/lmdb_map_size are now user-configurable, but invalid values can be passed through (e.g. lmdb_max_dbs=0 will later prevent creating named databases via Env::create_database(Some(name))). Consider validating these settings in Environment::new() and returning a clear client error (e.g. require max_dbs >= 1 and map_size > 0) before calling heed::EnvOpenOptions.

Addressed — validation is now handled on the Rust side with client_bail! per @georgeh0's guidance.

Copilot · 2026-02-24T20:32:04Z

python/cocoindex/_internal/setting.py

        )
        global_execution_options = GlobalExecutionOptions(**exec_kwargs)

+        lmdb_kwargs: dict[str, Any] = dict()


from_env() parses the LMDB settings with int, but does not validate the result. Negative/zero values (or extremely large values) will fail later during engine initialization/deserialization with less actionable errors. Consider validating that lmdb_max_dbs is >= 1 and lmdb_map_size is > 0 (and optionally within platform limits) and raising a ValueError with a clear message.

Copilot · 2026-02-24T20:32:04Z

python/cocoindex/_internal/setting.py

+        _load_field(
+            lmdb_kwargs,
+            "lmdb_max_dbs",
+            "COCOINDEX_LMDB_MAX_DBS",
+            parse=int,
+        )
+        _load_field(
+            lmdb_kwargs,
+            "lmdb_map_size",
+            "COCOINDEX_LMDB_MAP_SIZE",
+            parse=int,
+        )


New environment variables COCOINDEX_LMDB_MAX_DBS / COCOINDEX_LMDB_MAP_SIZE are introduced here, but there doesn’t appear to be test coverage asserting they’re honored by Settings.from_env(). Please add a unit/integration test that sets these env vars and verifies the resulting Settings carries the parsed values (and ideally that an Environment can be constructed with them).

Copilot · 2026-02-24T20:32:04Z

python/cocoindex/setting.py

+            "COCOINDEX_LMDB_MAP_SIZE",
+            parse=int,
+        )
+


from_env() parses the LMDB settings with int, but does not validate the result. Negative/zero values will fail later during engine initialization/deserialization with less actionable errors. Consider validating that lmdb_max_dbs is >= 1 and lmdb_map_size is > 0 and raising a ValueError with a clear message.

Suggested change

if "lmdb_max_dbs" in lmdb_kwargs:

lmdb_max_dbs = lmdb_kwargs["lmdb_max_dbs"]

if lmdb_max_dbs < 1:

raise ValueError(

f"COCOINDEX_LMDB_MAX_DBS must be an integer >= 1, got {lmdb_max_dbs}"

)

if "lmdb_map_size" in lmdb_kwargs:

lmdb_map_size = lmdb_kwargs["lmdb_map_size"]

if lmdb_map_size <= 0:

raise ValueError(

f"COCOINDEX_LMDB_MAP_SIZE must be an integer > 0, got {lmdb_map_size}"

)

Validation is now handled on the Rust side with client_bail! as suggested by @georgeh0, so Python-side validation is not needed.

georgeh0

Thanks for the PR!

georgeh0 · 2026-02-25T16:47:08Z

python/cocoindex/_internal/setting.py

+    lmdb_max_dbs: int | None = None
+    lmdb_map_size: int | None = None


I think we can put the default values at Python side (similar to GlobalExecutionOptions.source_max_inflight_rows). This will make the default more clear from Python API. Then these fields are always not None and we don't need to apply default by constants from Rust side.

Done in f2308d6 — moved defaults to the Python side, matching the source_max_inflight_rows pattern. Fields are now always concrete int values, never None. Removed Option<> wrappers on the Rust side as well, with #[serde(default = "...")] for backward compatibility.

georgeh0 · 2026-02-25T16:48:00Z

python/cocoindex/setting.py

FYI, this file is no longer used in v1 (I'll do a cleanup soon). It's OK to either keep or revert current changes.

Thanks for the heads up! I have kept the changes for consistency with _internal/setting.py. Happy to revert if you prefer — just let me know.

georgeh0 · 2026-02-25T16:53:06Z

rust/core/src/engine/environment.rs

+        let max_dbs = settings.lmdb_max_dbs.unwrap_or(DEFAULT_MAX_DBS);
+        let map_size = settings.lmdb_map_size.unwrap_or(DEFAULT_LMDB_MAP_SIZE);


To resolve the copilot comments on the setting.py, we can validate these values are positive here and raise a error otherwise (use client_bail!)

Done in f2308d6 — added validation with client_bail! before calling heed::EnvOpenOptions:

if settings.lmdb_max_dbs < 1 { client_bail!("lmdb_max_dbs must be >= 1, got {}", settings.lmdb_max_dbs); } if settings.lmdb_map_size == 0 { client_bail!("lmdb_map_size must be > 0, got {}", settings.lmdb_map_size); }

This also resolves the Copilot comments about Python-side validation — invalid values are now caught at the Rust layer with clear error messages.

- Move LMDB default values to Python side (lmdb_max_dbs=1024, lmdb_map_size=4GiB) so fields are never None, matching the pattern used by GlobalExecutionOptions.source_max_inflight_rows - Make Rust EnvironmentSettings fields non-optional (u32/usize) with serde default functions for backward compatibility - Add validation with client_bail! for invalid values (max_dbs < 1, map_size == 0) - Remove unused u32 import from std

georgeh0

Thanks for the PR! Merging.

georgeh0 · 2026-03-01T16:12:03Z

python/cocoindex/setting.py

+        lmdb_kwargs: dict[str, Any] = dict()
+        _load_field(
+            lmdb_kwargs,
+            "lmdb_max_dbs",
+            "COCOINDEX_LMDB_MAX_DBS",
+            parse=int,
+        )
+        _load_field(
+            lmdb_kwargs,
+            "lmdb_map_size",
+            "COCOINDEX_LMDB_MAP_SIZE",
+            parse=int,
+        )


It's good for now. Thanks!

Copilot AI review requested due to automatic review settings February 24, 2026 20:26

Copilot started reviewing on behalf of Harry-kp February 24, 2026 20:26 View session

Copilot AI reviewed Feb 24, 2026

View reviewed changes

Harry-kp marked this pull request as draft February 24, 2026 20:55

georgeh0 reviewed Feb 25, 2026

View reviewed changes

Harry-kp marked this pull request as ready for review February 27, 2026 19:02

georgeh0 approved these changes Mar 1, 2026

View reviewed changes

georgeh0 merged commit 5d1d0ec into cocoindex-io:v1 Mar 1, 2026
14 checks passed

Haleshot mentioned this pull request Mar 1, 2026

[FEATURE] Make LMDB max dbs and map size configurable #1703

Closed

github-actions bot mentioned this pull request Mar 6, 2026

[upstream-sync] Bootstrap: upstream changes 2026-01-25 to 2026-03-06 knitli/recoco#27

Closed

6 tasks

bashandbone mentioned this pull request Mar 11, 2026

[Upstream-Sync] **Phase 1 (Easy):** — Make LMDB max_dbs and map_size configurable (adopts upstream PR #1704) knitli/recoco#56

Closed

3 tasks

		@@ -5,13 +5,16 @@ use crate::{
		use serde::{Deserialize, Serialize};
		use std::{collections::BTreeSet, path::PathBuf, u32};

+        if "lmdb_max_dbs" in lmdb_kwargs:
+            lmdb_max_dbs = lmdb_kwargs["lmdb_max_dbs"]
+            if lmdb_max_dbs < 1:
+                raise ValueError(
+                    f"COCOINDEX_LMDB_MAX_DBS must be an integer >= 1, got {lmdb_max_dbs}"
+                )
+        if "lmdb_map_size" in lmdb_kwargs:
+            lmdb_map_size = lmdb_kwargs["lmdb_map_size"]
+            if lmdb_map_size <= 0:
+                raise ValueError(
+                    f"COCOINDEX_LMDB_MAP_SIZE must be an integer > 0, got {lmdb_map_size}"
+                )

		lmdb_max_dbs: int \| None = None
		lmdb_map_size: int \| None = None

		let max_dbs = settings.lmdb_max_dbs.unwrap_or(DEFAULT_MAX_DBS);
		let map_size = settings.lmdb_map_size.unwrap_or(DEFAULT_LMDB_MAP_SIZE);

Conversation

Harry-kp commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Usage

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

georgeh0 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

georgeh0 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Harry-kp commented Feb 24, 2026 •

edited

Loading