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

Add get_or_spawn method #328

Open
ihor-rud opened this issue Feb 3, 2025 · 5 comments
Open

Add get_or_spawn method #328

ihor-rud opened this issue Feb 3, 2025 · 5 comments
Labels
enhancement New feature or request

Comments

@ihor-rud
Copy link

ihor-rud commented Feb 3, 2025

Is your feature request related to a problem? Please describe.
I would like to get actor from the registry or create a new one if it is not present. Currently, it can be done by combining registry::where_is and Actor::spawn method with some synchronization on top of it.

Describe the solution you'd like
I can see 2 ways:

  1. add registry::get_or_spawn method
  2. expose Entry like api for registry for advanced customization
@ihor-rud ihor-rud added the enhancement New feature or request label Feb 3, 2025
@endic-sam928281
Copy link

Hello, we tried to solve the issue.

This is what we did:

Implemented a new get_or_spawn method in the registry module to allow users to get an actor from the registry or create a new one if it doesn't exist. This addresses the feature request for a more convenient way to handle actor retrieval and creation.

You can review changes in this commit: endic-sam928281@b4b4f34.

Caution

Disclaimer: The concept of solution was created by AI and you should never copy paste this code before you check the correctness of generated code. Solution might not be complete, you should use this code as an inspiration only.


Latta AI seeks to solve problems in open source projects as part of its mission to support developers around the world. Learn more about our mission at https://latta.ai/ourmission . If you no longer want Latta AI to attempt solving issues on your repository, you can block this account.

@slawlor
Copy link
Owner

slawlor commented Feb 4, 2025

If you no longer want Latta AI to attempt solving issues on your repository, you can block this account.

Wayyyy ahead of ya

@slawlor
Copy link
Owner

slawlor commented Feb 4, 2025

So I'm poking around how this might work, but specifically we don't want to allow writing to the registry externally. It's enclosed inside the actor lifecycle, specifically to avoid clashes. The means exposing something like an Entry API isn't really something I want from an architecture perspective.

So that leaves something like a get_or_init function, like what would be on a OnceCell. However again it would require re-creating all the startup arguments, etc. I'm worried this API would end up being clunky and annoying to work with.

The best option, I believe, is the current API. Given the concurrency guarantees, you do have a risk of getting a SpawnErr::ActorAlreadyRegistered error, but if you

  1. Check for existence
  2. If no, spawn new actor

You don't have any race conditions. If you need a wrapper on it, I believe it's best served by your own wrapper locally. Since there's a few spawn api's that would need to be accounted for (spawn, spawn_linked, with or without a name, etc). That would mean a lot of overloading or optional args, which ... meh?

But I'm open to comments and discussion!

@ihor-rud
Copy link
Author

ihor-rud commented Feb 4, 2025

So I'm poking around how this might work, but specifically we don't want to allow writing to the registry externally. It's enclosed inside the actor lifecycle, specifically to avoid clashes. The means exposing something like an Entry API isn't really something I want from an architecture perspective.

The Entry API can expose only lifecycle-related methods. For example, my shutdown logic for actors looks like this:

ractor::registry::registered()
    .into_iter()
    .filter_map(ractor::registry::where_is)
    .for_each(|actor| actor.stop(None));

which is not thread-safe. I have a feeling that the registry API should instead expose thread safe method for such usecase.

The best option, I believe, is the current API. Given the concurrency guarantees, you do have a risk of getting a SpawnErr::ActorAlreadyRegistered error, but if you

1. Check for existence

2. If no, spawn new actor

You don't have any race conditions.

This is not thread-safe. Two threads can look for an actor at the same time, and, not finding it, proceed to spawning. The whole point of the get_or_init method is to prevent this kind of race.

Since there's a few spawn api's that would need to be accounted for (spawn, spawn_linked, with or without a name, etc). That would mean a lot of overloading or optional args, which ... meh?

Maybe exposing a builder and a single spawn method will do the trick? Something like this:

Actor::spawn(actor, args, SpawnArgs::build().name("name").linked_to(other_actor));

Adding new methods to builder should not introduce breaking API changes.

@slawlor
Copy link
Owner

slawlor commented Feb 4, 2025

I'd be happy to look at a PR should you wish to introduce one. I'm not convinced at the moment but perhaps looking at how it might work would help.

Let me know if that's something you'd like to try out

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

No branches or pull requests

3 participants