feat(python): expose spillway controls (open kwargs + runtime setters) - #8
Merged
Conversation
….open()
The spillway feature landed on the Rust side 2026-05-04 and stabilized;
the Python binding was holding back with `spillway_max_bytes = 0` (the
v1-conservative "disable spillway entirely") hard-coded in src/db.rs.
This exposes both open-time options that the Rust side has shipped,
matching Rust's effective defaults.
Three changes:
1. New PyO3 pyclass enum `DrainInsertion` in src/db.rs mirroring
`chisel::DrainInsertion`. Two variants `LruTail` (default) and
`Mru`. Variant names match Rust spelling so cross-referencing
ARCHITECTURE.md ADR-5 stays mechanical. Registered on the module
via `m.add_class::<PyDrainInsertion>()`.
2. `chisel.open()` accepts two new keyword-only args:
- `spillway_max_bytes: int | None = None` — None resolves at
runtime to `1024 * cache_max_bytes` (Rust default; 8 GiB at the
8 MiB cache default). Explicit 0 disables and restores
CacheFull-at-cap. Sentinel-based default is required because
a literal default could not scale with a user-overridden
cache_max_bytes.
- `drain_insertion: DrainInsertion = DrainInsertion.LruTail` —
the natural Rust default. PyO3 routes the enum value through
a `From<PyDrainInsertion> for chisel::DrainInsertion` impl.
3. Removed the "v1 spillway disabled, deferred until Rust shipped
and stabilized" comment in db.rs::open — it was itself stale
the moment the spillway feature shipped on main.
Python-side updates:
- chisel/__init__.py: re-export DrainInsertion + add to __all__.
- chisel/chisel.pyi: add DrainInsertion class stub + update open()
signature.
- README.md: update "Opening a database" snippet; new "Cache and
spillway" section explaining the two new kwargs; flip the
SpillwayFullError row note from "reserved for future use" to
"now reachable when the spillway fills."
Tests (tests/test_open.py): 8 new pass — DrainInsertion smoke
checks (variants present, equality semantics) + open() acceptance
tests for spillway=None / spillway=0 / custom spillway / both
drain variants / in-memory with both kwargs.
Verification:
- maturin develop: clean (56 pre-existing PyO3 cfg warnings; no
new ones from this diff)
- pytest: 73/73 pass (65 pre-existing + 8 new)
- cargo clippy: 90 errors, matching baseline (the 1 `too_many_arguments`
introduced by adding two kwargs is suppressed by a narrow #[allow]
on open() with an explanatory comment — keyword-only args can't be
confused by position, and bundling 8 kwargs into a struct would
be less ergonomic than kwargs in Python)
- rustfmt: src/db.rs clean (the transaction.rs fmt diff is
pre-existing on main and untouched by this PR)
Runtime config setters (set_cache_max_bytes / set_spillway_max_bytes
/ set_drain_insertion) intentionally NOT exposed in this PR — the
review only asked about open-time kwargs. Defer that decision.
…n_insertion
Companion to the open-time kwarg exposure earlier in this PR — mirrors
the three between-transaction configuration mutators that already exist
on chisel::Chisel. The setters share the existing with_inner_mut_io
helper, so they inherit:
- ClosedError if called on a closed handle (closed/poisoned check
centralised in one place, matching every other mutating method)
- PoisonedError if the manager is poisoned from an earlier fatal
- TransactionInProgressError if called mid-transaction (which is
finally a reachable error class on the Python side; until this
commit, no API path could produce it)
Methods on PyChisel:
- set_cache_max_bytes(bytes: int) -> None
- set_spillway_max_bytes(bytes: int) -> None
- set_drain_insertion(policy: chisel.DrainInsertion) -> None
Drain-insertion takes the same PyDrainInsertion pyclass enum already
introduced earlier in this PR; PyO3's From<PyDrainInsertion> for
chisel::DrainInsertion impl handles the conversion to the Rust side.
Tests in a new tests/test_runtime_config.py (9 cases): happy-path
for each setter, mid-transaction raises TransactionInProgressError
for each setter, a "failed setter doesn't disturb the active
transaction" check (operational error → engine survives), and a
"closed handle" check confirming the existing closed-detection path
applies uniformly.
README adds a "Runtime configuration" subsection under "Opening a
database" walking through the setter use case + the mid-transaction
restriction. The error-table row for TransactionInProgressError is
flipped from "reserved for future use" to citing the three setters
that now produce it.
Verification:
- maturin develop: clean
- pytest: 82/82 pass (73 prior + 9 new in test_runtime_config.py)
- cargo clippy: 93 errors. 90 are pre-existing PyO3/clippy version-
boundary noise (`useless_conversion to the same type: pyo3::PyErr`
on every pymethod's PyResult return type). The +3 from this commit
are three more instances of the identical pattern — the new setters
follow the exact same signature shape every existing pymethod uses.
Real fix is at the PyO3 version layer, out of scope.
- rustfmt src/db.rs: clean
🚦 Bench results: PR vs main
Per-scenario detail (4 metrics × cells)document-store
mutation-log
ycsb-a
ycsb-b
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces #7 (which was auto-closed when its base branch `claude/python-docs-staleness-catchup` was deleted by PR #6's merge — same head branch, rebased onto current `main`).
Summary
Closes the deliberate v1 omission of spillway controls from the Python binding. The spillway feature shipped on Chisel's `main` 2026-05-04 and stabilized; the binding was holding back with `spillway_max_bytes = 0` (disabled) hard-coded in `python/src/db.rs`. That deferral is now redeemed.
What's exposed (commit-by-commit)
Commit 1: open-time kwargs
```python
chisel.open(
path,
cache_max_bytes=8_388_608, # unchanged
spillway_max_bytes=None, # NEW: None → 1024 × cache_max_bytes
drain_insertion=chisel.DrainInsertion.LruTail, # NEW
create_if_missing=True,
read_only=False,
superblock_count=2,
)
```
Commit 2: between-transaction setters on PyChisel
```python
db.set_cache_max_bytes(4 * 1024 * 1024)
db.set_spillway_max_bytes(0)
db.set_drain_insertion(chisel.DrainInsertion.Mru)
```
All three operate ONLY on between-transactions state — calling any of them mid-transaction raises `TransactionInProgressError` (which is finally a reachable error class on the Python side).
Design choices
Test plan
Diff scope
```
python/README.md | +46 / -8
python/chisel/init.py | +2 / -1
python/chisel/chisel.pyi | +18 / -0
python/src/db.rs | +84 / -11
python/tests/test_open.py | +66 / -0
python/tests/test_runtime_config.py | +114 / -0 (new file)
```