Skip to content

Semi-private python API for overriding handle_undeliverable_message inside PythonActor #797

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

samlurye
Copy link

@samlurye samlurye commented Aug 7, 2025

Summary: This diff makes undeliverable message handling overridable for python actors, using the newly introduced Actor._handle_undeliverable_message method. Previously, the rust implementation of PythonActor simply used the default Actor::handle_undeliverable_message implementation. Now, PythonActor overrides handle_undeliverable_message to call into the corresponding method on the underlying python class.

Differential Revision: D79841379

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Aug 7, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D79841379

samlurye pushed a commit to samlurye/monarch-1 that referenced this pull request Aug 7, 2025
…nside PythonActor (meta-pytorch#797)

Summary:

This diff makes undeliverable message handling overridable for python actors, using the newly introduced `Actor._handle_undeliverable_message` method. Previously, the rust implementation of `PythonActor` simply used the default `Actor::handle_undeliverable_message` implementation. Now, `PythonActor` overrides `handle_undeliverable_message` to call into the corresponding method on the underlying python class.

Differential Revision: D79841379
samlurye pushed a commit to samlurye/monarch-1 that referenced this pull request Aug 7, 2025
…nside PythonActor (meta-pytorch#797)

Summary:

This diff makes undeliverable message handling overridable for python actors, using the newly introduced `Actor._handle_undeliverable_message` method. Previously, the rust implementation of `PythonActor` simply used the default `Actor::handle_undeliverable_message` implementation. Now, `PythonActor` overrides `handle_undeliverable_message` to call into the corresponding method on the underlying python class.

Differential Revision: D79841379
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D79841379

samlurye pushed a commit to samlurye/monarch-1 that referenced this pull request Aug 7, 2025
…nside PythonActor (meta-pytorch#797)

Summary:
Pull Request resolved: meta-pytorch#797

This diff makes undeliverable message handling overridable for python actors, using the newly introduced `Actor._handle_undeliverable_message` method. Previously, the rust implementation of `PythonActor` simply used the default `Actor::handle_undeliverable_message` implementation. Now, `PythonActor` overrides `handle_undeliverable_message` to call into the corresponding method on the underlying python class.

Differential Revision: D79841379
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D79841379

samlurye pushed a commit to samlurye/monarch-1 that referenced this pull request Aug 7, 2025
…nside PythonActor (meta-pytorch#797)

Summary:
Pull Request resolved: meta-pytorch#797

This diff makes undeliverable message handling overridable for python actors, using the newly introduced `Actor._handle_undeliverable_message` method. Previously, the rust implementation of `PythonActor` simply used the default `Actor::handle_undeliverable_message` implementation. Now, `PythonActor` overrides `handle_undeliverable_message` to call into the corresponding method on the underlying python class.

Differential Revision: D79841379
Summary:
The purpose of this diff is to handle the following scenario:
1. Process A starts serving a NetRx.
2. Process B creates a NetTx that connects to process A's NetRx.
3. B sends a few messages to A, and the messages are acked.
4. Process A dies/is killed, while B stays alive.
5. A new Process C starts serving a NetRx on the same channel as from step 1.
6. B's NetTx connects to C's NetRx, *with no way of knowing it has connected to a different process than before*.
7. B sends messages to C, starting from where it left off with A.
8. C rejects all of B's messages because of invalid sequence numbers.
9. B's NetTx eventually times out after a long time with no acks.

In order to distinguish among connections from different NetTx instances to the same NetRx instance, each NetTx generates a random unique session id. This session id gets sent as part of an initial handshake from NetTx -> NetRx before the NetTx starts sending normal messages.

Currently, though, NetTx doesn't wait for any handshake before starting to send messages. To resolve the issue described above, this diff introduces a global (per-process) "rx session id". When a NetTx first connects to a NetRx, the NetRx responds with its rx session id as part of the handshake. The NetTx waits for the handshake response and extracts the rx session id. If this is the first time the NetTx is connecting, the NetTx stores the rx session id. On subsequent connection attempts, the NetTx will validate the rx session id it receives from the handshake against the rx session id it previously stored; if there is a mismatch, the NetTx returns the appropriate error to its caller.

Differential Revision: D79607092
samlurye pushed a commit to samlurye/monarch-1 that referenced this pull request Aug 8, 2025
…nside PythonActor (meta-pytorch#797)

Summary:

This diff makes undeliverable message handling overridable for python actors, using the newly introduced `Actor._handle_undeliverable_message` method. Previously, the rust implementation of `PythonActor` simply used the default `Actor::handle_undeliverable_message` implementation. Now, `PythonActor` overrides `handle_undeliverable_message` to call into the corresponding method on the underlying python class.

Differential Revision: D79841379
samlurye pushed a commit to samlurye/monarch-1 that referenced this pull request Aug 8, 2025
…nside PythonActor (meta-pytorch#797)

Summary:

This diff makes undeliverable message handling overridable for python actors, using the newly introduced `Actor._handle_undeliverable_message` method. Previously, the rust implementation of `PythonActor` simply used the default `Actor::handle_undeliverable_message` implementation. Now, `PythonActor` overrides `handle_undeliverable_message` to call into the corresponding method on the underlying python class.

Differential Revision: D79841379
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D79841379

samlurye pushed a commit to samlurye/monarch-1 that referenced this pull request Aug 8, 2025
…nside PythonActor (meta-pytorch#797)

Summary:
Pull Request resolved: meta-pytorch#797

This diff makes undeliverable message handling overridable for python actors, using the newly introduced `Actor._handle_undeliverable_message` method. Previously, the rust implementation of `PythonActor` simply used the default `Actor::handle_undeliverable_message` implementation. Now, `PythonActor` overrides `handle_undeliverable_message` to call into the corresponding method on the underlying python class.

Differential Revision: D79841379
…nside PythonActor (meta-pytorch#797)

Summary:
Pull Request resolved: meta-pytorch#797

This diff makes undeliverable message handling overridable for python actors, using the newly introduced `Actor._handle_undeliverable_message` method. Previously, the rust implementation of `PythonActor` simply used the default `Actor::handle_undeliverable_message` implementation. Now, `PythonActor` overrides `handle_undeliverable_message` to call into the corresponding method on the underlying python class.

Differential Revision: D79841379
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D79841379

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot. fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants