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

Registered subjects #45

Open
dvic opened this issue May 27, 2024 · 8 comments
Open

Registered subjects #45

dvic opened this issue May 27, 2024 · 8 comments

Comments

@dvic
Copy link

dvic commented May 27, 2024

The README of gleam/otp mentions

There is no support for named processes. They are untyped global mutable variables which may be uninitialized, more research is needed to find a suitable type safe alternative.

I just wanted to drop a thought here and see if we can further explore a solution to this.

We now have the Subject, which is an opaque type:

pub opaque type Subject(message) {
  Subject(owner: Pid, tag: Reference)
}

As far as I understood is that the current approach to using supervised actors is to send the subject to the parent proces. I propose that we keep it like that, but instead of new_subject in the actor we do something like new_local_subject(name) and have

pub opaque type Subject(message) {
  // no name registration
  Subject(owner: Pid, tag: Reference)
  // for node-local name registration
  LocalSubject(name: Atom, tag: Reference)
  // for cluster-wide global name registration
  GlobalSubject(name: Atom, tag: Reference)
}

So basically everything remains the same but we don't need to handle actor restarts in our code explicitly: when the actor restarts it will register again under the same name.

Local, global and possible in the future custom registration can be done like in erlang:

https://github.com/erlang/otp/blob/79bc8234396a4f619f40dadfb8458064ab29aa85/lib/stdlib/src/gen.erl#L622-L640

Are there any problems with this solution?

@dvic dvic changed the title name registered subjects Registered subjects May 27, 2024
@lpil
Copy link
Member

lpil commented Jun 2, 2024

Hello! Thanks for bringing this up, I've been thinking about this a bunch too over the last few years :)

This design enables a subject to effectively be inherited by a new process, removing the need to propagate a subject when a process is restarted/replaced.

It does not remove that need to get the subject the first time round, like Erlang is able to with its names. I think that's likely OK!

With this addition it would be possible for a subject to not have a pid. We would need to make the function for getting that return an option?

It would also mean that actor.start could technically succeed but not return a pid. Do we want to rethink the API there? For example, we might want to make it return a pid and also some other data rather than always a subject. This would also enable us to return other data from the start function, that would be cool.

For global names I'm not sure how we would be able to construct them in a type safe way. Perhaps this should be left for later as it's much more complex.

@dvic
Copy link
Author

dvic commented Jun 2, 2024

This design enables a subject to effectively be inherited by a new process, removing the need to propagate a subject when a process is restarted/replaced.

Yes!

It does not remove that need to get the subject the first time round, like Erlang is able to with its names. I think that's likely OK!

Agreed :)

With this addition it would be possible for a subject to not have a pid. We would need to make the function for getting that return an option?

Yes, or what named now returns (a Result(Pid, Nil):

/// Look up a process by name, returning the pid if it exists.
///
@external(erlang, "gleam_erlang_ffi", "process_named")
pub fn named(name: Atom) -> Result(Pid, Nil)

The function subject_owner would then pattern match on the actual type and call named for the local registered kind.

However, this would then be a breaking change, not sure how you feel about that?

It would also mean that actor.start could technically succeed but not return a pid. Do we want to rethink the API there? For example, we might want to make it return a pid and also some other data rather than always a subject. This would also enable us to return other data from the start function, that would be cool.

Why? I was thinking of keeping the API the same and return a StartError if new_local_subject(name: Atom) fails (it tries to immediately register).

For global names I'm not sure how we would be able to construct them in a type safe way. Perhaps this should be left for later as it's much more complex.

Isn't it the same but then just calling global:register_name, like in erlang (see below)? Or are you worried about crossing node boundaries and hitting code version differences?

image

@lpil
Copy link
Member

lpil commented Jun 2, 2024

Another thing we may want think about is how we deal with sending to a named subject where the process is dead. Should that result in an error? For call maybe we want an error type with an error variant that calls out this specific situation.

However, this would then be a breaking change, not sure how you feel about that?

We're definitely going to need to make some breaking changes here, so I think it's good to think about what else we would want to break here.

Some candidates on my mind at the moment are:

  1. Making it easier for processes to pass data back up the tree after they are started.
  2. Making it possible to supervise non-Gleam processes by having the child spec return a pid rather than a subject.
  3. Renaming "supervisor" to something else to make it clear that it is only one of many possible supervisors. "rest_for_one_supervisor"?

Is there anything else?

Isn't it the same but then just calling global:register_name, like in erlang (see below)? Or are you worried about crossing node boundaries and hitting code version differences?

Yes. In a distributed scenario there is no guarantee that the same code is running on both nodes. Worse: it commonly will not be the same.

@dvic
Copy link
Author

dvic commented Jun 3, 2024

Another thing we may want think about is how we deal with sending to a named subject where the process is dead.

Erlang does throw an error in this case as the docs of send/2 say:

The function fails with a badarg run-time error if Dest is an atom name, but this name is not registered. This is the only case when send fails for an unreachable destination Dest (of correct type).

For monitoring erlang does not throw an error but instead sends a :noproc DOWN message.

Should that result in an error? For call maybe we want an error type with an error variant that calls out this specific situation.

We already kind of have an error for this right? The type CallError has variant CalleeDown(reason: Dynamic) and it's used in try_call. So maybe we can make the reason typed? Or make reason optional?

However, this would then be a breaking change, not sure how you feel about that?

We're definitely going to need to make some breaking changes here, so I think it's good to think about what else we would want to break here.

Clear. I will play around with a few ideas and submit a draft PR for the changes in gleam/erlang so we can shoot at it, it makes the discussion easier I think.

Some candidates on my mind at the moment are:

  1. Making it easier for processes to pass data back up the tree after they are started.
  2. Making it possible to supervise non-Gleam processes by having the child spec return a pid rather than a subject.
  3. Renaming "supervisor" to something else to make it clear that it is only one of many possible supervisors. "rest_for_one_supervisor"?

Sounds good! But these changes you're talking have to be made in gleam/otp right? For now I was just focusing on gleam/erlang but I do agree we should think about how these changes fit together.

Is there anything else?

Not that I can think of now.

Isn't it the same but then just calling global:register_name, like in erlang (see below)? Or are you worried about crossing node boundaries and hitting code version differences?

Yes. In a distributed scenario there is no guarantee that the same code is running on both nodes. Worse: it commonly will not be the same.

Clear.

@dvic
Copy link
Author

dvic commented Jun 5, 2024

@lpil I have a first try on the named-subjects branch, apart from introducing new_named_subject:

// Create a new `Subject` owned by the current process and registered with
/// the given name. This function may fail for the same reasons as `register/2`.
///
pub fn new_named_subject(name: Atom) -> Result(Subject(message), Nil) {
  let register_result = register(self(), name)
  result.map(register_result, fn(_) {
    Subject(destination: NamedDestination(name), tag: erlang.make_reference())
  })
}

it introduces a SubjectOwner type, with two breaking changes:

  • subject owner returns a SubjectOwner instead of Pid
pub type SubjectOwner {
  PidDestination(Pid)
  NamedDestination(Atom)
}
  • send returns a Result(message, Nil)

image

Do you think this is a direction that makes sense?

@lpil
Copy link
Member

lpil commented Jun 5, 2024

Sounds good! But these changes you're talking have to be made in gleam/otp right? For now I was just focusing on gleam/erlang but I do agree we should think about how these changes fit together.

We can't think about changes in the context of just one package, we need to think about the entire ecosystem before making any changes here as it's such an important part of Gleam.

subject owner returns a SubjectOwner instead of Pid

The point of the function is to return the pid so it must continue to return a pid. Otherwise everyone has to write the same boilerplate case expression around this function, which isn't very useful.

Do you think [result returning send] is a direction that makes sense?

I'm not sure. It is strange to me that it returns ok if it is a pid and the process is dead, but an error if it is a name and it is dead.

@dvic
Copy link
Author

dvic commented Jun 5, 2024

We can't think about changes in the context of just one package, we need to think about the entire ecosystem before making any changes here as it's such an important part of Gleam.

Makes sense 👍

The point of the function is to return the pid so it must continue to return a pid. Otherwise everyone has to write the same boilerplate case expression around this function, which isn't very useful.

You mean Result(Pid, Nil), right, because in case of a registered name the lookup might fail?

I'm not sure. It is strange to me that it returns ok if it is a pid and the process is dead, but an error if it is a name and it is dead.

Yes, I was also thinking of this. This is currently what erlang does (sending to dead pid -> no error, sending to unregistered name -> error), that's why I was following it. But I can see the benefits of unifying this. So then we would check for the 'liveness' for both the registered and unregistered processes?

@lpil
Copy link
Member

lpil commented Jul 10, 2024

Hello! Sorry I'm taking so long here, I'm rather swamped. I've not had time to read your branch but I hope to soon.

I don't think we should copy from Erlang for the error cases, rather we want to have an API that is designed for Gleam's goals and constraints.

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

No branches or pull requests

2 participants