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

Actors v18 #755

Merged
merged 128 commits into from
Sep 6, 2023
Merged

Actors v18 #755

merged 128 commits into from
Sep 6, 2023

Conversation

vyzo
Copy link
Collaborator

@vyzo vyzo commented Aug 6, 2023

Actors v18; see #724

This is a rewrite of the std/actor package, with a leaner and more powerful protocol, a much more ergononic API, and a practical management tool.
The legacy package has been deprecated and moved to std/actor-v13 until it gets removed in some future release.

See the documentation for more details.

@vyzo vyzo changed the base branch from master to bio-delimited-util August 6, 2023 14:08
@vyzo vyzo mentioned this pull request Aug 6, 2023
21 tasks
Base automatically changed from bio-delimited-util to master August 17, 2023 10:29
@vyzo vyzo changed the title [WIP] Actors v18 Actors v18 Aug 17, 2023
@vyzo vyzo marked this pull request as ready for review August 17, 2023 10:34
@vyzo vyzo requested review from fare, ober and drewc August 17, 2023 10:34
@vyzo
Copy link
Collaborator Author

vyzo commented Aug 17, 2023

rebased and dropped the npm changes as they break the documentation build; oh well.

@vyzo vyzo mentioned this pull request Aug 19, 2023
@vyzo vyzo force-pushed the actors-v18 branch 2 times, most recently from b2601a6 to f80416b Compare August 23, 2023 19:13

### rpc.unmonitor
::: tip usage
### remote-stop-server!
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe remote-server-stop!?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have stop-actor! above where stop is a prefix, but here stop is the suffix. Maybe

stop-actor! -> actor-stop!
remote-stop-server! -> remote-server-stop!

for unified names?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stop-remote-server! probably more consistent; let me think about this.

I use the remote prefix in a few places to make it obvious this is a remote action.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remote? Doesn't it work the same local and remote?

@vyzo vyzo added this to the Gerbil18 milestone Aug 27, 2023
@vyzo vyzo force-pushed the actors-v18 branch 2 times, most recently from bf332d7 to 2393295 Compare August 31, 2023 06:35
,(@shutdown
(infof "loader shutting down ...")
(exit 'shutdown))
,(@ping)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoa, this is wild, and deserves a note here, since the <- is only defined later.
Does match also have it? Is there a macro to help my macros have it too? etc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

match doesn't have it, I came up with the gnostic syntax later -- maybe we should add it? It is really nice for composition indeed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is very nice, and something I've wanted for a long time. I was surprised it was possible, and what more with regular syntax-rule!

@vyzo
Copy link
Collaborator Author

vyzo commented Sep 5, 2023

rebased on master; it's in the future now.

Copy link
Collaborator

@fare fare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I had only a cursory look at the code itself, but LGTM modulo remarks for documentation.

Internet. It allows servers to find each other by using the server
identifier, and implicit connect as needed without requiring the
programmer or the operator to explicitly connect to each other.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that keep working if my IP address changes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you'd need to update the server address in the registry; you can do this with ensemble-add-server!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will my laptop even know it has changed address? That might happen often as I hop between wifi and mobile... at which point my IP might often be firewalled.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, if you are running an ensemble server in your laptop exactly what are you advertising?
It is far more likely that you are running a client in your laptop, in which case you don't have to do anything.

Having said that, if you want to run a server in your laptop, we could make a separate utility to detect changing IPs and update the registry entry.
I simply don't think this is a concern at this level, and we have all the necessary procedures to write such a tool.

to authenticate each other. The cookie is placed in `$GERBIL_PATH/ensemble/cookie`;
if you are running an ensemble spanniong multiple hosts, you should copy the cookie
to the relevant hosts. Note that the tool will not overwrite an existing ensemble
cookie.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Race condition?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, safety; it would be bad to accidentally override your cookie.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, there is an inherent race condition when two processes both try to create the cookie. Are we trying very hard to avoid it? How?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that it is reasonable to create cookies programmatically and shoot yourself in the foot.
This should be an administrative step that should run once, either manually or as part of your ensemble container/tarball/rsync preparation process.

So I don't think we need to do anything about it.

@vyzo vyzo merged commit 7988ab0 into master Sep 6, 2023
@vyzo vyzo deleted the actors-v18 branch September 6, 2023 11:05
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