Skip to content
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

Fix Gateway <-> Wormhole interop #294

Merged
merged 2 commits into from
Nov 20, 2023
Merged

Conversation

ettersi
Copy link
Collaborator

@ettersi ettersi commented Nov 17, 2023

Consider a setup where a Java Gateway is connected to a master container with a Unet Wormhole agent running in it. In this scenario, the gateway will currently not receive any messages addressed to it.

The technical reasons behind this are as follows.

  • The gateway's name is of the form Gateway@<number>, i.e. it looks just like an agent that is wormholed to the master container.
  • The Java gateway currently does not submit a wantsMessagesFor to the master container. Therefore, it receives all the messages that aren't claimed on the master container, but it also receives only those.
  • Consequently, the gateway doesn't receive messages addressed to it: the wormhole claims all messages addressed to <name>@<number>, and the master container doesn't know that the gateway would like to receive messages addressed to Gateway@<number>.

This PR fixes both of these aspects.

  • It makes sure that Java Gateways do send a wantsMessagesFor on startup.
  • It changes the gateway naming pattern from Gateway@(\d+) to Gateway-(\d+) to avoid confusion with wormholed agents.

Strictly speaking, either change would have been enough on its own, but there's no reason not to fix both for extra robustness (and performance in the case of the wantsMessagesFor).

@ettersi ettersi requested a review from mchitre November 17, 2023 02:36
@ettersi ettersi self-assigned this Nov 17, 2023
@notthetup
Copy link
Collaborator

Do we need to update the other language Gateways? And maybe update the Gateway spec?

@mchitre
Copy link
Member

mchitre commented Nov 19, 2023

Good question. Need to check the naming of the gateways from other languages.

@ettersi ettersi merged commit 5e3dcc6 into master Nov 20, 2023
2 checks passed
@ettersi ettersi deleted the slave-container-update-watchlist branch November 20, 2023 01:45
@ettersi
Copy link
Collaborator Author

ettersi commented Nov 20, 2023

@mchitre Any chance we can make a release with this change? ST needs this for Aquarius, and making a release is probably the cleanest way to ship this code to them.

@mchitre
Copy link
Member

mchitre commented Nov 20, 2023

Yes, let me try to do that sometime today

@mchitre
Copy link
Member

mchitre commented Nov 20, 2023

Released fjage-1.12.1

@ettersi
Copy link
Collaborator Author

ettersi commented Nov 20, 2023

Do we need to update the other language Gateways? And maybe update the Gateway spec?

JS, Python and Julia gateways are fine. I've fixed the C gateway in #295.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants