Modular transports via a tractor.ipc subpkg! #24

Closed
goodboy wants to merge 53 commits from leslies_extra_appendix into shm_apis

That’s right, we’re finally not just using TCP!

Bo


This originally started as a appendix PR to #17 and was thus named,

Refinements around multiple IPC transport protos (an appendix to #17)

Now this PR will replace #17 since the history is to be fast forwarded onto the structural_dynamics_of_flow branch originally started by @guille (with much thanks!)

The original description was,

A variety of simplifying-refinements and fixes to the core runtime due to issues discovered as part of making our entire test suite work over both TCP and UDS sockets!


PR land plan

I’d like to see this landed before the original upstream branch (#10) which contains the new (and much more experimental) high-perf shared-mem .ipc._ringbug stuff from @guille.

  • that branch should get rebased onto this one eventually and landed after all the linux-specific additions have been worked out in detail.

So our first common commit between this and #10 should be,

  • 9b21615 Break out transport protocol and tcp specifics into their own submodules

From the original history I’ve dropped the following commits that can instead go with #10:

  • bab265b Important RingBuffBytesSender fix on non batched mode!
  • 010874b Catch trio cancellation on RingBuffReceiver bg eof listener task
  • a010ab Add direct read method on EventFD
  • be7fc89 Add direct ctx managers for RB channels
  • 2a9a786 Improve test_ringbuf test
  • be818a7 Switch tractor.ipc.MsgTransport.stream type to trio.abc.Stream
  • ba353bf Better encapsulate RingBuff ctx managment methods

Amendments to tractor.ipc and ._addr vs. original #17

  • Much simplification to new ._addr.Address primitives,

    • use a type alias UnwrappedAddress,
    • drop all the Address type-var inheritance,
    • drop Address.open_stream(), never used.
    • make UDSAddress.unwrap() to a tuple[str, str] of the (._filedir, ._filename) pair, effectively the (Path.parent, Path.name) components of a (new) .sockpath: Path.
    • rename class vars to .proto_key and unwrapped_type.
    • redo namespace as .bindspace to both better reflect the socket.bind() argument significance as well as avoid collision with the linux semantics of “network namespaces” (which i’d like to utilize eventually!).
    • add .def_bindspace attrs to each concrete type.
  • Changes to UDSAddress.bindspace: Path semantic,

    • use a new _state.get_rt_dir() as the default directory-value representing (in the same vein as a net-proto ip-address) the “basis of the bind space”, that is for socket-files which will be created under that directory-node.
      • This is more equivalent to ports available to be bound per routing-proto address such that file-names like ports only have collision for a given address-part.
      • That is, filename (equivalent of port numbers) only can experience a file-name collision when the files are under the same parent directory (in the mount-namespace on linux) for the process.
    • do appropriate type-matching in wrap_address() and .from_addr() to match.
    • use the PID and if available Actor.aid info in the socket file name to indicate that process is the likely .listen()er/server for that socket/file.
    • mk .close_listener() sync.
  • moved the _addr.preferred_transport to _state._def_tpt_proto: TransportProtocolKey = 'tcp'

  • extend ipc._uds for peer-credentials reading from the OS,

    • add a new open_unix_socket_w_passcred() which ensures with sock.setsockopt() that the server side can always read the connecting client’s pid.
    • add a get_peer_info() helper to read that^
    • always return UDSAddresss with the .maybe_pid set from the peer on the raddr for groking/introspection/repr purposes!
  • new deps introduced,

    • bidict for a 2way map ._address_types: [str, Type[Address]].

Changes to surrounding core components

  • add and use a new Actor.aid: Aid:

    • switch Channel.aid/uid to it^
    • make a new optional Aid.pid: int field for getting any peer’s process id much more easily!
    • always render the .uuid field from a new _addr.mk_uuid().
  • add real __repr__ = pformat representations to,

    • Actor,
    • Channel,
    • MsgPackTransport (well ish, it’s a start),
  • move Actor._do_handshake() to Channel method.

  • proper/rigorous SpawnSpec handling in Actor._from_parent() such that we actually raise an explicit internal error on type-mismatches (and not some weird downstream error..).

  • add some new runtime excs,

    • RuntimeFailure for out of our control init issues.
    • ActorFailure now from that for failed spawns.
  • add a new .devx.pformat.pformat_exc() factored from the body of RemoteActorError.pformat() and used for TransportClosed.

  • factor out breakpoint() blocking to a _root.maybe_block_bp(); should be factored to .devx maybe ya?


Full test suite multi-tpt support with --tpt-proto=uds|tcp

I was able to augment the entire test to opt into multi-transport-protocols via a new accumulative --tpt-proto flag which by default is always just stored as ['tcp'] as before.

Now any opting in test/fixture can pass it to the enable_transports: list[str] argument exposed by open_root_actor() to test against that backend; currently we haven’t tried to test multiple protos together yet though!

brief test harness updates summary

new conftest fixture content,

  • a new tpt_protos: list[str] rendered from CLI input args.
  • tpt_proto: str which just takes the first of the above for now.
  • reg_addr now correctly generates a random root-host-singleton-registry value per test session/run.
  • masked draft-idea for a set_script_runtime_args().

test suite tweaks for UDS,

  • test_advanced_faults.py adjustments for receiving non-KBIs since the socket “closure”/“breakage” will be relayed by the OS immediately.
    • also matching against the new TransportClosed.src_exc field for the raised underlying trio exceptions.

a new test_root_runtime.py suite with,

  • test_only_one_root_actor to verify we error on opening a 2nd root from an actor tree.
  • test_implicit_root_via_first_nursery which just verifies implicit root/runtime booting from the first opened .open_nursery().

New --dev deps introduced

  • psutil for proc introspection within the discovery suite.

Follow up refinements I’d like to maybe see before #17 lands,

As in after this is merged into #17, I’d like to see these additional changes considered,


GH issues landing #17 will maybe close!

### That's right, we're finally not just using TCP! Bo --- This originally started as a appendix PR to #17 and was thus named, > Refinements around multiple IPC transport protos (an appendix to #17) Now this PR will replace #17 since the history is to be fast forwarded onto the `structural_dynamics_of_flow` branch originally started by @guille (with much thanks!) The original description was, >A variety of simplifying-refinements and fixes to the core > runtime due to issues discovered as part of making our entire test > suite work over both TCP and UDS sockets! --- PR land plan ------------ I'd like to see this landed before the original upstream branch (#10) which contains the new (and much more experimental) high-perf shared-mem `.ipc._ringbug` stuff from @guille. - [ ] that branch should get rebased onto this one eventually and landed after all the linux-specific additions have been worked out in detail. So our first common commit between this and #10 should be, - 9b21615 Break out transport protocol and tcp specifics into their own submodules From the original history I've dropped the following commits that can instead go with #10: - [x] bab265b Important RingBuffBytesSender fix on non batched mode! - [x] 010874b Catch trio cancellation on RingBuffReceiver bg eof listener task - [x] a010ab Add direct read method on EventFD - [x] be7fc89 Add direct ctx managers for RB channels - [x] 2a9a786 Improve test_ringbuf test - [x] be818a7 Switch `tractor.ipc.MsgTransport.stream` type to `trio.abc.Stream` - [x] ba353bf Better encapsulate RingBuff ctx managment methods --- Amendments to `tractor.ipc` and `._addr` vs. original #17 --------------------------------------------------------- - Much simplification to new `._addr.Address` primitives, - use a type alias `UnwrappedAddress`, - drop all the `Address` type-var inheritance, - drop `Address.open_stream()`, never used. - make `UDSAddress.unwrap()` to a `tuple[str, str]` of the `(._filedir`, `._filename)` pair, effectively the `(Path.parent, Path.name)` components of a (new) `.sockpath: Path`. - rename class vars to `.proto_key` and `unwrapped_type`. - redo `namespace` as `.bindspace` to both better reflect the `socket.bind()` argument significance as well as avoid collision with the linux semantics of "network namespaces" (which i'd like to utilize eventually!). - add `.def_bindspace` attrs to each concrete type. - Changes to `UDSAddress.bindspace: Path` semantic, - use a new `_state.get_rt_dir()` as the default directory-value representing (in the same vein as a net-proto ip-address) the "basis of the bind space", that is for socket-files which will be created *under* that directory-node. * This is more equivalent to ports available to be bound per routing-proto address such that file-names like ports only have collision for a given address-part. * That is, filename (equivalent of port numbers) only can experience a file-name collision when the files are under the same parent directory (in the mount-namespace on linux) for the process. - do appropriate type-matching in `wrap_address()` and `.from_addr()` to match. - use the PID and if available `Actor.aid` info in the socket file name to indicate that process is the likely `.listen()`er/server for that socket/file. - mk `.close_listener()` sync. - moved the `_addr.preferred_transport` to `_state._def_tpt_proto: TransportProtocolKey = 'tcp'` - extend `ipc._uds` for peer-credentials reading from the OS, - add a new `open_unix_socket_w_passcred()` which ensures with `sock.setsockopt()` that the server side can always read the connecting client's pid. - add a `get_peer_info()` helper to read that^ - always return `UDSAddress`s with the `.maybe_pid` set from the peer on the `raddr` for groking/introspection/repr purposes! - new deps introduced, - `bidict` for a 2way map `._address_types: [str, Type[Address]]`. --- Changes to surrounding core components -------------------------------------- - add and use a new `Actor.aid: Aid`: - switch `Channel.aid/uid` to it^ - make a new optional `Aid.pid: int` field for getting any peer's process id much more easily! - always render the `.uuid` field from a new `_addr.mk_uuid()`. - add real `__repr__ = pformat` representations to, - `Actor`, - `Channel`, - `MsgPackTransport` (well ish, it's a start), - move `Actor._do_handshake()` to `Channel` method. - proper/rigorous `SpawnSpec` handling in `Actor._from_parent()` such that we actually raise an explicit internal error on type-mismatches (and not some weird downstream error..). - add some new runtime excs, - `RuntimeFailure` for out of our control init issues. - `ActorFailure` now from that for failed spawns. - add a new `.devx.pformat.pformat_exc()` factored from the body of `RemoteActorError.pformat()` and used for `TransportClosed`. - factor out `breakpoint()` blocking to a `_root.maybe_block_bp()`; should be factored to `.devx` maybe ya? --- Full test suite multi-tpt support with `--tpt-proto=uds|tcp` ------------------------------------------------------------ I was able to augment the entire test to opt into multi-transport-protocols via a new accumulative `--tpt-proto` flag which by default is always just stored as `['tcp']` as before. Now any opting in test/fixture can pass it to the `enable_transports: list[str]` argument exposed by `open_root_actor()` to test against that backend; currently we **haven't tried to test multiple protos together yet though!** #### brief test harness updates summary new `conftest` fixture content, - a new `tpt_protos: list[str]` rendered from CLI input args. - `tpt_proto: str` which just takes the first of the above for now. - `reg_addr` now correctly generates a random root-host-singleton-registry value per test session/run. - masked draft-idea for a `set_script_runtime_args()`. test suite tweaks for UDS, - `test_advanced_faults.py` adjustments for receiving non-KBIs since the socket "closure"/"breakage" will be relayed by the OS immediately. - also matching against the new `TransportClosed.src_exc` field for the raised underlying `trio` exceptions. a new `test_root_runtime.py` suite with, - `test_only_one_root_actor` to verify we error on opening a 2nd root from an actor tree. - `test_implicit_root_via_first_nursery` which just verifies implicit root/runtime booting from the first opened `.open_nursery()`. New `--dev` deps introduced - `psutil` for proc introspection within the discovery suite. --- Follow up refinements I'd like to maybe see before #17 lands, ------------- As in after this is merged into #17, I'd like to see these additional changes considered, - [x] if we can move each concrete protocol's `Address` into its `.ipc/_tcp/uds` module. - as of - [ ] consider where we want the final destination of `._addr` since there's going to be extensions necessary as we start solving the multi-addresss idea and composition of tpt protos from addresses as kinda requested in https://github.com/goodboy/tractor/pull/367 with further follow up as we research the libp2p spec-docs: * https://github.com/libp2p/specs/blob/master/addressing/README.md#addressing-in-libp2p-- * https://github.com/multiformats/multiaddr - [ ] factor the transport serving into a new `TransportServer` (singleton?-)wrapper which keeps track of all connections (`SocketStream`s wrapped by `MsgTransport`) per protocol and peer! - [ ] as part of this removing the `.open/close_listener()` meths from per-tpt addrs-types and instead offering the api at module level for each proto? --- GH issues landing #17 will maybe close! ------------- - [ ] https://github.com/goodboy/tractor/issues/109 - [ ] https://github.com/goodboy/tractor/issues/136
goodboy changed title from Refinements around multiple IPC transport protos, and appendix for #17 to Refinements around multiple IPC transport protos (an appendix to #17) 2025-04-06 20:33:07 +00:00
goodboy reviewed 2025-04-06 22:24:41 +00:00
tractor/_addr.py Outdated
@ -48,2 +97,4 @@
# Lke, what use does this have besides a noop and if it's not
# valid why aren't we erroring on creation/use?
@property
def is_valid(self) -> bool:
Poster
Owner

i’m not really sure how useful this predicate is tbh.

i'm not really sure how useful this predicate is tbh.
goodboy reviewed 2025-04-06 22:28:22 +00:00
tractor/_addr.py Outdated
@ -157,3 +267,1 @@
async def open_listener(self, **kwargs) -> trio.SocketListener:
listeners = await trio.open_tcp_listeners(
async def open_listener(
Poster
Owner

i think maybe we should consider moving this to MsgpackTransport as well?

If it provides an interface around SocketStream-like instances (for each peer pair) i think it makes more sense to maybe do something like,

await MsgpackUDSStream.open_listener(addr=UDSAddress.get_random()) instead of pulling weird Address()-instance-internal state?

i think maybe we should consider moving this to `MsgpackTransport` as well? If it provides an interface around `SocketStream`-like instances (for each peer pair) i think it makes more sense to maybe do something like, `await MsgpackUDSStream.open_listener(addr=UDSAddress.get_random())` instead of pulling weird `Address()`-instance-internal state?
goodboy reviewed 2025-04-06 22:34:05 +00:00
tractor/_addr.py Outdated
@ -176,3 +285,1 @@
trio.SocketStream,
trio.SocketListener
]):
def unwrap_sockpath(
Poster
Owner

just a helper for any UDSAddress..sockpath: Path style partitioning to dir, filename.

just a helper for any `UDSAddress..sockpath: Path` style partitioning to dir, filename.
goodboy reviewed 2025-04-06 22:40:12 +00:00
tractor/_root.py Outdated
@ -67,3 +147,3 @@
# defaults are above
arbiter_addr: tuple[AddressTypes]|None = None,
arbiter_addr: tuple[UnwrappedAddress]|None = None,
Poster
Owner

REALLY need to drop this!

XD

i’m hoping to get to a follow up registry-sys rework this week in a follow up PR!

REALLY need to drop this! XD i'm hoping to get to a follow up registry-sys rework this week in a follow up PR!
goodboy reviewed 2025-04-06 22:41:04 +00:00
tractor/_root.py Outdated
@ -61,0 +72,4 @@
# save an indent level?
#
@acm
async def maybe_block_bp(
Poster
Owner

NOTE most of the diff in this file is just the higher indent of open_root_actor() due to factoring this bp blocking stuff out!

NOTE most of the diff in this file is just the higher indent of `open_root_actor()` due to factoring this bp blocking stuff out!
goodboy reviewed 2025-04-06 22:42:02 +00:00
@ -202,3 +202,1 @@
self.uid = (
name,
uid or str(uuid.uuid4())
self._aid = msgtypes.Aid(
Poster
Owner

Notice we always stash the pid now!

Notice we always stash the pid now!
goodboy reviewed 2025-04-06 22:49:38 +00:00
@ -1199,0 +1342,4 @@
if (
'[Errno 98] Address already in use'
in
oserr.args#[0]
Poster
Owner

drop that #[0] bit..

drop that `#[0]` bit..
goodboy reviewed 2025-04-06 22:51:03 +00:00
@ -146,0 +152,4 @@
_rtdir: Path = Path(os.environ['XDG_RUNTIME_DIR'])
def get_rt_dir(
Poster
Owner

used for our UDSAddress.def_bindspace: path

used for our `UDSAddress.def_bindspace: path`
goodboy requested review from guille 2025-04-06 23:01:51 +00:00
guille approved these changes 2025-04-07 01:44:19 +00:00
goodboy force-pushed leslies_extra_appendix from 44fd1ecc41 to 8fd7d1cec4 2025-04-07 02:33:15 +00:00 Compare
goodboy changed title from Refinements around multiple IPC transport protos (an appendix to #17) to Modular transports via a `tractor.ipc` subpkg! 2025-04-07 02:58:56 +00:00
goodboy changed title from Modular transports via a `tractor.ipc` subpkg! to Modular transports via a `tractor.ipc` subpkg! 2025-04-07 03:00:41 +00:00
goodboy changed target branch from structural_dynamics_of_flow to shm_apis 2025-04-07 03:00:42 +00:00
Poster
Owner

Wow it seems i can’t update the pull_from branch here!?

that’s like nuts..

I guess if it can’t be done i’m going to have to open a new PR.. or just land this branch i guess..

Wow it seems i can't update the `pull_from` branch here!? that's like nuts.. I guess if it can't be done i'm going to have to open a new PR.. or just land this branch i guess..
goodboy force-pushed leslies_extra_appendix from 800c99ac41 to c9e9a3949f 2025-04-08 14:10:12 +00:00 Compare
goodboy added 1 commit 2025-04-11 03:18:50 +00:00
c208bcbb1b Factor actor-embedded IPC-tpt-server to `ipc` subsys
Primarily moving the `Actor._serve_forever()`-task-as-method and
supporting actor-instance attributes to a new `.ipo._server` sub-mod
which now encapsulates,
- the coupling various `trio.Nursery`s (and their independent lifetime mgmt)
  to different `trio.serve_listener()`s tasks and `SocketStream`
  handler scopes.
- `Address` and `SocketListener` mgmt and tracking through the idea of
  an "IPC endpoint": each "bound-and-active instance" of a served-listener
  for some (varied transport protocol's socket) address.
- start and shutdown of the entire server's lifetime via an `@acm`.
- delegation of starting/stopping tpt-protocol-specific `trio.abc.Listener`s
  to the corresponding `.ipc._<proto_key>` sub-module (newly defined
  mod-top-level instead of `Address` method) `start/close_listener()`
  funcs.

Impl details of the `.ipc._server` sub-sys,
- add new `IPCServer`, allocated with `open_ipc_server()`, and which
  encapsulates starting multiple-transport-proto-`trio.abc.Listener`s
  from an input set of `._addr.Address`s using,
  |_`IPCServer.listen_on()` which internally spawns tasks that delegate to a new
    `_serve_ipc_eps()`, a rework of what was (effectively)
    `Actor._serve_forever()` and which now,
    * allocates a new `IPCEndpoint`-struct (see below) for each
      address-listener pair alongside the specified
      listener-serving/stream-handling `trio.Nursery`s provided by the
      caller.
    * starts and stops each transport (socket's) listener by calling
      `IPCEndpoint.start/close_listener()` which in turn delegates to
      the underlying `inspect.getmodule(IPCEndpoint.addr)` backend tpt
      module's equivalent impl.
    * tracks all created endpoints in a `._endpoints: list[IPCEndpoint]`
      which is further exposed through public properties for
      introspection of served transport-protocols and their addresses.
  |_`IPCServer._[parent/stream_handler]_tn: Nursery`s which are either
     allocated (in which case, as the same instance) or provided by the
     caller of `open_ipc_server()` such that the same nursery-cancel-scope
     controls offered by `trio.serve_listeners(handler_nursery=)` are
     offered where the `._parent_tn` is used to spawn `_serve_ipc_eps()`
     tasks, and `._stream_handler_tn` is passed verbatim as `handler_nursery`.
- a new `IPCEndpoint`-struct (as mentioned) which wraps each
  transport-proto's address + listener + allocated-supervising-nursery
  to encapsulate the "lifetime of a server IPC endpoint" such that
  eventually we can track and managed per-protocol/address/`.listen_on()`-call
  scoped starts/stops/restarts for the purposes of filtering/banning
  peer traffic.
  |_ also included is an unused `.peer_tpts` table which we can
    hopefully use to replace `Actor._peers` in a `Channel`-tracking
    transport-proto-aware way!

Surrounding changes to `.ipc.*` primitives to match,
- make `[TCP|UDS]Address` types `msgspec.Struct(frozen=True)` and thus
  drop any-and-all `addr._host =` style mutation throughout.
  |_ as such also drop their `.__init__()` and `.__eq__()` meths.
  |_ UDS tweaks to field names and thus `.__repr__()`.
- move `[TCP|UDS]Address.[start/close]_listener()` meths to be mod-level
  equiv `start|close_listener()` funcs.
- just hard code the `.ipc._types._key_to_transport/._addr_to_transport`
  table entries instead of all the prior fancy dynamic class property
  reading stuff (remember, "explicit is better then implicit").

Modified in `._runtime.Actor` internals,
- drop the `._serve_forever()` and `.cancel_server()`, methods and
  `._server_down` waiting logic from `.cancel_soon()`
- add `.[_]ipc_server` which is opened just after the `._service_n` and
  delegate to it for any equivalent publicly exposed instance
  attributes/properties.
Poster
Owner

This is now replaced by an equiv PR from the branch from #17 onto shm_apis,

https://github.com/goodboy/tractor/pull/375

Once i move desired content from the descr here to there i will be closing this one.

This is now replaced by an equiv PR from the branch from #17 onto `shm_apis`, https://github.com/goodboy/tractor/pull/375 Once i move desired content from the descr here to there i will be closing this one.
Poster
Owner
Landed in https://github.com/goodboy/tractor/pull/375
goodboy closed this pull request 2025-10-06 16:24:18 +00:00

Pull request closed

Sign in to join this conversation.
No reviewers
No Label
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: goodboy/tractor#24
There is no content yet.