feat(maru): add pool_id config for dax device selection#6
feat(maru): add pool_id config for dax device selection#6hyunyul-XCENA wants to merge 9 commits into
Conversation
- Add use_layerwise guard (NotImplementedError) - Change zip strict=False to strict=True in batched_submit_put_task - Add warning log when contains(pin=True) is called - Add warning log for in-flight put_tasks on close()
…or MaruBackend Enable MaruBackend to participate in StorageManager.async_lookup_and_prefetch() by implementing the two required async lookup APIs. Both use asyncio.to_thread to wrap sync RPC calls (handler.exists / handler.retrieve) without blocking the event loop.
- Convert maru:// to tcp:// in _create_handler for ZMQ compatibility - Call memory_obj.pin() in get_blocking to match cleanup unpin, fixing pin_count=-1 warning on retrieve path
Previously _async_store called on_complete_callback in the finally block regardless of success/failure, which could signal false success to callers and mask CXL page leaks on store failure. Aligns with LocalCPUBackend and NixlDynamic which only call callback on success.
Read maru_pool_id from extra_config and pass to MaruConfig.
Supports int, list[int], and comma-separated string formats.
Usage in LMCache YAML:
extra_config:
maru_pool_id: 0 # single pool
maru_pool_id: [0, 1] # fallback chain
maru_pool_id: "0,1,2" # comma-separated
seohui-XCENA
left a comment
seohui-XCENA
left a comment
There was a problem hiding this comment.
Clean and minimal change. The dict + **kwargs pattern for conditionally passing pool_id is appropriate, and it follows the existing extra_config conventions.
| if isinstance(pool_id, list): | ||
| maru_kwargs["pool_id"] = [int(p) for p in pool_id] | ||
| elif isinstance(pool_id, str) and "," in pool_id: | ||
| maru_kwargs["pool_id"] = [int(p.strip()) for p in pool_id.split(",")] |
There was a problem hiding this comment.
[Minor] No error handling on int() conversion.
Malformed input (e.g. "abc", [1, "x"]) will raise a bare ValueError and crash MaruBackend initialization. _parse_pool_size in the same file uses try/except ValueError with a fallback, so this is inconsistent.
That said, for pool_id a silent fallback could be dangerous (mapping to the wrong dax device), so fail-fast is arguably better — but with a clear error message:
try:
maru_kwargs["pool_id"] = int(pool_id)
except (ValueError, TypeError) as e:
raise ValueError(f"Invalid maru_pool_id={pool_id!r}: {e}") from eThere was a problem hiding this comment.
Fixed in 9897fce. Wrapped all int() conversions with try/except (ValueError, TypeError) and re-raise with clear message:
raise ValueError(f"Invalid maru_pool_id={pool_id!r}: {e}") from eAgreed that fail-fast is better than silent fallback for pool_id — wrong dax device mapping would be worse than a crash.
| eager_map=extra.get("maru_eager_map", True), | ||
| ) | ||
| pool_id = extra.get("maru_pool_id") | ||
| if pool_id is not None: |
There was a problem hiding this comment.
[Nit] Empty list / empty string edge cases.
maru_pool_id: []→isinstance(pool_id, list)is true →maru_kwargs["pool_id"] = []maru_pool_id: ""→ falls toelse: int("")→ValueErrormaru_pool_id: ","→split(",")→["", ""]→int("")→ValueError
Empty values could be treated the same as None (i.e. skip and let maru_resourced decide).
There was a problem hiding this comment.
Fixed in 9897fce. Empty values now treated as None (skip):
[]→if pool_id:guard, skips""→if stripped:guard, skips","→if p.strip()filter in list comprehension, produces empty → skips
All result in maru_resourced deciding the pool.
- Wrap int() conversions in try/except with clear ValueError message - Handle empty list, empty string, and comma-only string as None (skip) - Filter out empty entries in comma-separated format
hyunyul-XCENA
left a comment
hyunyul-XCENA
left a comment
There was a problem hiding this comment.
Addressed both review comments in 9897fce:
[Minor] int() error handling:
Wrapped all int() conversions with try/except (ValueError, TypeError) and re-raise as ValueError with clear message. Agreed that fail-fast is better than silent fallback for pool_id.
[Nit] Empty edge cases:
Empty values now treated as None (skip):
[]→if pool_id:guard, skips""→if stripped:guard, skips","→if p.strip()filter in list comprehension, produces empty → skips
|
나중에 extra config에 별도의 device path 같은 걸 넣어서 사용할 수 있게 하면 될 듯 합니다. 일단 홀딩. |
3 participants
Summary
maru_pool_idfromextra_configand pass it toMaruConfig.pool_idint,list[int], and comma-separated string formatsUsage
Changes
maru_backend.py:_create_handler()readsextra_config.maru_pool_idand passes it toMaruConfigkwargsconfig.py— follows existingextra_configpattern used by other maru options