feat: out-of-tree handler plugin system (maru.handler_plugins)#58
Open
hyunyul-XCENA wants to merge 9 commits into
Open
feat: out-of-tree handler plugin system (maru.handler_plugins)#58hyunyul-XCENA wants to merge 9 commits into
hyunyul-XCENA wants to merge 9 commits into
Conversation
Let vendor-/hardware-specific behaviour live outside maru core instead of in a separate package. MaruHandler discovers plugins registered under the `maru.handler_plugins` entry-point group at construction and calls them at four seams: on_init, on_batch_retrieve, on_close, contribute_stats. Design follows vLLM's soft-fail plugin model (not PyTorch's hard-fail autoload): a Maru plugin is an optional optimization, so a load/hook failure is logged and skipped, never raised. MARU_PLUGINS filters by name. Plugins couple to core only through a small stable accessor surface: - MaruHandler.is_region_mapped(region_id) - MaruHandler.get_region_dax_path(region_id) (via mapper/client get_dax_path) Everything a plugin needs about a batch arrives via the on_batch_retrieve hook args. Empty-plugin hot path stays free (guarded dispatch). Adds maru_handler/plugin.py (MaruHandlerPlugin protocol + soft-fail loader), tests/unit/test_plugin_loader.py, and a README Plugins section.
Mark MaruHandler.is_region_mapped / get_region_dax_path and the four MaruHandlerPlugin hook signatures as the public, stable plugin contract that out-of-tree plugins depend on across independent release cycles. - docs/source/api_reference/plugins.md: entry-point group, hook table with timing, stable accessor surface, soft-fail + version-skew warning, MARU_PLUGINS. - handler.py: strengthen accessor docstrings/comment to flag them as stable API. - test_plugin_loader.py: contract test pinning the accessor signatures, hook names, and entry-point group name so CI fails if the surface drifts.
…allowlist Review-driven robustness for configuration mistakes (soft-fail already covers runtime exceptions): - a factory returning None is skipped (was retained as a dead "plugin") - duplicate entry-point names: first wins + warn (running both would double every hook — matches vLLM's loader) - an allowlist (MARU_PLUGINS) name matching no installed plugin now warns instead of silently loading nothing Adds unit tests for all three (16 passed).
Main's get_stats now returns stats.stats_manager; update the plugin get_stats test's RPC stub so it doesn't AttributeError after the rebase.
ea88784 to
c68c29c
Compare
Public maru docs shouldn't name a specific vendor plugin. Replace the named example with a generic 'writing a plugin' description and genericize the region-mapping note.
youngrok-XCENA
left a comment
Collaborator
There was a problem hiding this comment.
OOT plugin 시스템 설계·구현 품질이 매우 높습니다 (soft-fail 로더, 좁은 커플링 표면, 계약 테스트, vendor-neutral 문서). 블로킹 이슈는 없습니다.
요청하신 대로 MEDIUM 2건 · LOW 2건을 인라인으로 남깁니다:
- MEDIUM-1 (
plugins.md): "owned region은 hint 안 쏨" 서술이 실제 코드와 배치 (owned도 mapper에 등록됨) - MEDIUM-2 (
test_plugin_loader.py): 계약 테스트가on_batch_retrieve필드 표면을 pin하지 않음 - LOW-1 (
handler.py): 매__init__마다 entry-point 스캔 - LOW-2 (
handler.py): 서버 제공 offset/length가 plugin으로 무검증 전달 (deferred 확인)
MEDIUM 2건은 머지 전 처리를 권합니다.
…cache EP scan - docs(MEDIUM-1): correct the plugins.md note — owned regions ARE registered in the mapper (owned_region_manager.add_region → mapper.map_region), so on_batch_retrieve fires on owned regions too; a plugin wanting shared-only must filter (get_owned_region_ids). The prior note asserted a false owned-is-skipped safety property. - test(MEDIUM-2): contract test now also pins the on_batch_retrieve payload fields (BatchLookupKVResponse.entries, LookupResult.found/handle/kv_offset/ kv_length, MaruHandle.region_id/offset) — the real cross-package contract a maru_common rename would silently break. - perf(LOW-1): cache the maru.handler_plugins entry-point scan at module level (installed set is immutable per process) instead of scanning every MaruHandler.__init__. LOW-2 (bounds-check server-supplied offset/length before device ops) tracked as #59.
…l_pool) #60 (feat: expose CXL pool free_size in get_stats) added stats.cxl_pool to get_stats(); the plugin get_stats test's RPC stub was missing it → CI AttributeError. Mirror the stats_manager fix.
Tighten the MaruHandlerPlugin hook docstrings, the public API this PR establishes, per review findings: - on_batch_retrieve: KV bytes are at entry.kv_offset/kv_length; handle.offset is only the region mmap base, so a HW hint must target handle.offset + kv_offset. Also note batch_retrieve is not serialized against close(), so a hint can land after unmap — plugins must be thread-safe and idempotent. - on_close: fires only if the handler was connected (not paired with on_init); runs under the write lock, so it must not block. - on_init: not paired with on_close — do not acquire there a resource whose release depends on on_close. - contribute_stats: keyed by class name, so same-named plugins collide. - Class docstring: soft-fail isolates raised exceptions, not hangs. Docstring-only; no behavior change. ruff clean, plugin loader tests pass.
Collaborator
Author
|
Follow-up in 6775e2a: folded the remaining review findings into the stable hook contract as docstring-only hardening (no behavior change).
ruff clean, 17 plugin loader tests pass. LOW-2 (server offset/length bounds-check) remains tracked in #59. |
4 tasks
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
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.
Summary
Adds an out-of-tree (OOT) handler plugin system so vendor-/hardware-specific behaviour can live in separate packages instead of Maru core. Maru stays vendor-neutral: a plugin package registers a
maru.handler_pluginsentry point andMaruHandlerdiscovers it at construction and calls it at defined seams — core never imports it. Loading is soft-fail (vLLM-style), so a missing or broken plugin never breaks KV-cache operation.Key Changes
maru_handler/plugin.py(new):MaruHandlerPluginprotocol (4 optional hooks) +load_handler_plugins()soft-fail loader — filters byMARU_PLUGINS, dedups duplicate names (first wins), and warns on aNonefactory return / typo'd allowlist name.maru_handler/handler.py: dispatch at four seams (on_init/on_batch_retrieve/on_close/contribute_stats) via_dispatch_plugins; the empty-plugin case is guarded so the retrieval hot path stays free.on_batch_retrievefires after the region-mapping loop (a plugin acts on regions mapped in the current batch); plugin stats are namespaced understats["plugins"][<PluginClassName>].MaruHandler.is_region_mapped()andget_region_dax_path()(backed by newmapper/clientget_dax_path). A contract test fails CI if the accessor signatures, hook names, or entry-point group drift.docs/source/api_reference/plugins.md.kv_offset + kv_lengthbefore device ops; access-dumper flush/logging tuning.How it works
Core discovers plugins purely through the entry point, calls them at four lifecycle seams, and a plugin reaches back into core only through the hook arguments plus two documented accessors — nothing else couples the two packages.
sequenceDiagram autonumber participant EP as entry points<br/>(maru.handler_plugins) participant H as MaruHandler (core) participant PL as Plugin<br/>(separate package) participant HW as Device / hardware Note over H,PL: construction — MaruHandler.__init__ H->>EP: load_handler_plugins() — soft-fail, MARU_PLUGINS filter EP-->>H: plugin instance(s) H->>PL: on_init(handler) Note over H,PL: retrieval hot path — batch_retrieve(keys) H->>H: batch_lookup_kv → map shared regions H->>PL: on_batch_retrieve(handler, keys, batch_resp) loop found + mapped entries PL->>H: is_region_mapped(rid) / get_region_dax_path(rid) PL->>HW: device hint (prefetch / pin) end H-->>H: return MemoryInfo[] Note over H,PL: teardown — close() H->>PL: on_close(handler) PL->>HW: release device resourcesmaru.handler_pluginsentry point; a missing or broken plugin is logged and skipped (soft-fail).on_batch_retrieveargs plusis_region_mapped/get_region_dax_path— the stable contract a CI test pins.on_batch_retrieveruns after regions are mapped, so a plugin can act on live shared-memory addresses; with no plugin installed the seam is a single guarded no-op.Test Plan
tests/unit/test_plugin_loader.py(loader soft-fail / allowlist / dedup / None-return / dispatch isolation / API contract): 16 passed, ruff cleanpytest -v) — note: the full unit suite hits a pre-existing, environment-specific native crash in the mockedconnect()coverage tests on CUDA-enabled hosts (reproduces onmainwithout this branch; unrelated to this change)Related Issues