feat: make indexing and compile concurrency configurable#174
Open
KylinMountain wants to merge 6 commits into
Open
feat: make indexing and compile concurrency configurable#174KylinMountain wants to merge 6 commits into
KylinMountain wants to merge 6 commits into
Conversation
…rrency
PageIndex fans out one indexing LLM call per structure node; on a large
document that can open a socket per node and exhaust the process
file-descriptor limit ("[Errno 24] Too many open files"). This wires an
OpenKB config knob through to PageIndex's IndexConfig.max_concurrency cap.
- New `pageindex_max_concurrency` KB config key (default None = let PageIndex
apply its own default).
- indexer forwards it via `_build_index_config` only when set AND the installed
PageIndex's IndexConfig declares the field, so OpenKB keeps working against a
pinned PageIndex that predates it (IndexConfig forbids unknown kwargs).
Concept/entity generation ran at a hardcoded concurrency of 5 (DEFAULT_COMPILE_CONCURRENCY) with no way to lower it when the LLM provider rate-limits. Add a `compile_concurrency` config key (default 5) and thread it through every add / recompile / cloud-import compile call via a shared `_compile_concurrency` resolver (null / non-positive → the compiler default). Pairs with `pageindex_max_concurrency` (indexing side) from this PR — both concurrency knobs are now KB-configurable.
KylinMountain
changed the title
- resolve_compile_concurrency moves to config.py (matching resolve_timeout's convention) and now rejects bool (bool is an int subclass, so `compile_concurrency: true` previously silently became concurrency=1) and logs a warning on any malformed value, same as the other resolvers. - _build_index_config now warns when pageindex_max_concurrency is configured but the installed PageIndex doesn't support the field yet, instead of silently dropping it with no signal to the user. - Replaced the tautological test_forwards_max_concurrency_when_supported (which branched on the same runtime condition as the code under test, so it never exercised the forwarding assertion under CI's pinned PageIndex) with fake IndexConfig doubles that make both branches deterministic regardless of the installed pageindex version. - Added a cross-check test pinning DEFAULT_CONFIG["compile_concurrency"] against the compiler's own DEFAULT_COMPILE_CONCURRENCY so the two literals can't silently drift apart. - Added an end-to-end test proving index_long_document's own loaded config (not just a hand-built dict) reaches PageIndexClient's IndexConfig. - Hoisted the compile-concurrency resolution out of `recompile --all`'s per-document loop (was recomputed every iteration despite being loop-invariant). - Documented both new config.yaml keys in config.yaml.example and examples/configuration/README.md (kept in sync).
…ncurrency # Conflicts: # openkb/cli.py # tests/test_add_command.py
…cy into `concurrency` PageIndex indexing and OpenKB's own concept/entity compilation never run concurrently with each other for the same document (they're sequential phases of one add), and both knobs exist for the exact same reason — the user hit a provider rate limit or an fd ceiling. Splitting them by internal subsystem leaked OpenKB's architecture into the config surface for no practical benefit, since a user tuning one for a rate limit would set the other to the same value anyway. - Single `concurrency` config key (default null = each stage keeps its own built-in default). - config.resolve_concurrency(config) -> int | None: validates (rejects bool and non-positive values with a warning), returns None on unset/invalid — no default substitution inside; callers decide what None means for them. - cli.py's compile call sites: `resolve_concurrency(config) or DEFAULT_COMPILE_CONCURRENCY`. - indexer.py's _build_index_config now routes through resolve_concurrency (openkb validates before forwarding to PageIndex, rather than relying solely on PageIndex's own future validator) instead of raw config.get. - This also removes the prior dual-literal-default drift risk entirely: there's no second numeric default to keep in sync, since an unset `concurrency` simply forwards nothing to PageIndex and lets the compiler use its own DEFAULT_COMPILE_CONCURRENCY. - Updated config.yaml.example and examples/configuration/README.md (kept in sync) to the single key.
…urrency) 0.3.0.dev2 is the first published release that declares IndexConfig.max_concurrency, so `concurrency` in config.yaml now takes effect for the indexing stage in a normal `pip install` / `uv sync`, not just against a local editable checkout. Vetted: downloaded the wheel and confirmed its sha256 matches PyPI (b0cb1f6e…) and that IndexConfig declares `max_concurrency: int | None` with a positivity field_validator. uv.lock regenerated via `uv lock --upgrade-package pageindex`. The runtime `"max_concurrency" in IndexConfig.model_fields` guard in indexer._build_index_config is now always-true under the pinned dependency, but is kept as graceful degradation for mismatched local installs.
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.
Makes both concurrency knobs in the ingest pipeline KB-configurable, so users on rate-limited LLM providers (or a low file-descriptor limit) can tune them.
1.
compile_concurrency— compile step (closes #173)Concept/entity generation ran at a hardcoded concurrency of 5 (
DEFAULT_COMPILE_CONCURRENCY) with no way to lower it. #173 asks for it to be configurable for providers with rate limiting.compile_concurrencyconfig key (default 5).add/recompile/ cloud-import compile call via a shared_compile_concurrency(config)resolver (null / non-positive → compiler default).2.
pageindex_max_concurrency— PageIndex indexing stepLong-document indexing fans out one LLM call per structure node with no cap, which on a large document can open a socket per node and exhaust the process file-descriptor limit:
pageindex_max_concurrencyconfig key (defaultNone→ PageIndex's own default).indexer._build_index_config()forwards it to PageIndex'sIndexConfig.max_concurrencyonly when set AND the installed PageIndex declares the field (IndexConfighasextra="forbid"), so OpenKB keeps working against the pinned PageIndex that predates it.IndexConfig.max_concurrency(bump the pin in a follow-up).Testing
TestBuildIndexConfig,_compile_concurrencyresolution, an add-path integration test asserting the configured value reachescompile_short_doc, plus config default/override tests.ruff check/ruff formatclean.