-
Notifications
You must be signed in to change notification settings - Fork 778
Shard LCM stream channels across dedicated multicast buses. DIM - 1337 #3207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,14 +26,15 @@ | |
|
|
||
| import dimos.protocol.pubsub.impl.lcmpubsub as lcm | ||
| from dimos.protocol.pubsub.impl.lcmpubsub import PickleLCM | ||
| from dimos.protocol.service.lcmservice import lcm_url_for_channel | ||
|
|
||
|
|
||
| def test_publish_messages() -> None: | ||
| """Publish test messages to verify agentspy is working.""" | ||
| print("Starting agent message publisher demo...") | ||
|
|
||
| # Create transport | ||
| transport = PickleLCM() | ||
| # Create transport on the same sharded bus agentspy subscribes to | ||
| transport = PickleLCM(url=lcm_url_for_channel("/agent")) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You took over decisions on what an URL is, and this is now an protocol level internal variable. users should not be exposed to it at all, URL as a value is effectively killed. We can replace this as a namespace value or something, a number, but it cannot be a URL anymore. (actually yes, "namespace" can effectively be your "port range" - more on this below) Actual topic setting, a line below, will decide the URL you use, it is not known at the instantiation. This means that one pubsub instance can hold multiple LCM instances per topic on different ports |
||
| topic = lcm.Topic("/agent") | ||
|
|
||
| print(f"Publishing to topic: {topic}") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -193,17 +193,37 @@ def __init__(self, **lcm_kwargs: Any) -> None: | |
| # Inline import: an unavailable LCM backend must not break the other | ||
| # spy sources (see default_sources). | ||
| from dimos.protocol.pubsub.impl.lcmpubsub import LCMPubSubBase | ||
| from dimos.protocol.service.lcmservice import lcm_bus_urls | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we actually spread each topic per port, spying becomes a problem. I would actually ignore this problem as a first PR, just measure efficiency.. later, since we are implementing this as a separate protocol, we can have a simple meta topic that each instance can use to announce the topic it's on or something similar, can we somehow detect ports in use? and can auto-spy on all of them within our ELCM range |
||
| self._bus = LCMPubSubBase(**lcm_kwargs) | ||
| urls = (lcm_kwargs["url"],) if "url" in lcm_kwargs else lcm_bus_urls() | ||
| self._buses = [LCMPubSubBase(**{**lcm_kwargs, "url": url}) for url in urls] | ||
|
|
||
| def start(self) -> None: | ||
| self._bus.start() | ||
| started: list[Any] = [] | ||
| try: | ||
| for bus in self._buses: | ||
| bus.start() | ||
| started.append(bus) | ||
| except BaseException: | ||
| for bus in reversed(started): | ||
| bus.stop() | ||
| raise | ||
|
|
||
| def stop(self) -> None: | ||
| self._bus.stop() | ||
| for bus in self._buses: | ||
| bus.stop() | ||
|
|
||
| def tap(self, callback: Callable[[str, int], None]) -> Callable[[], None]: | ||
| return self._bus.subscribe_all(lambda msg, topic: callback(str(topic), len(msg))) | ||
| unsubs = [ | ||
| bus.subscribe_all(lambda msg, topic: callback(str(topic), len(msg))) | ||
| for bus in self._buses | ||
| ] | ||
|
|
||
| def unsubscribe() -> None: | ||
| for unsub in unsubs: | ||
| unsub() | ||
|
|
||
| return unsubscribe | ||
|
|
||
| def subscribe_decoded(self, topic: str, callback: Callable[[Any], None]) -> Callable[[], None]: | ||
| """Opt-in per-topic decoded tap — OFF the spy hot path. Not implemented in v1.""" | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why so conservative? this is a main knob that gives us efficiency, ideally we want each topic on its own port, I'd actually reserve a port range 50000-60000 or something, Allow user to exclude specific ports or redefine the range, But then just build the deterministic hash function. It takes a topic name and gives you a port number.