[Refactoring] send messages to dedicated actor instead of directly accessing objects

igalic commented

2020-07-16 19:45:59 +00:00

Owner

Instead of passing around references to directly access worker, and searcher, we could instead send messages to an actor!

This would also help split responsibility of these functions, as they would no longer invoke these actions themselves, only sending messages to invoke them.

This would allow to remove searcher and worker from our PlumeRocket. Together with #797, this could help completely dissolve the struct.

/// Common context needed by most routes and operations on models
pub struct PlumeRocket {
    pub conn: DbConn,
    pub user: Option<users::User>,
    pub searcher: Arc<search::Searcher>,
    pub worker: Arc<ScheduledThreadPool>,
}

How?

We could use an actor system, like riker.
Using multiple messages we could send different messages to different actors that then do what needs to be done

Instead of passing around references to directly access `worker`, and `searcher`, we could instead send messages to an actor! This would also help split responsibility of these functions, as they would no longer invoke these actions themselves, only sending messages to invoke them. This would allow to remove searcher and worker from our PlumeRocket. Together with #797, this could help completely dissolve the struct. ```rust /// Common context needed by most routes and operations on models pub struct PlumeRocket { pub conn: DbConn, pub user: Option<users::User>, pub searcher: Arc<search::Searcher>, pub worker: Arc<ScheduledThreadPool>, } ``` ### How? We could use an actor system, like [riker](https://riker.rs/). Using [multiple messages](https://riker.rs/messaging/) we could send different messages to different actors that then do what needs to be done

igalic referenced this issue

2020-07-17 07:41:31 +00:00

upgrade rocket* to the latest stable #800

kiwii referenced this issue from a commit

2020-07-17 13:39:12 +00:00

upgrade rocket* to the latest stable

kiwii commented

2020-07-17 13:42:23 +00:00

Owner

Honestly, it makes me wonder if actix wouldn't be more appropriate for our use case… we would get actors and a web framework out of the box. I think we already tried to migrate once and it didn't went very far, right? What was blocking?

But I'm okay with refactoring our code with Riker and keeping Rocket too.

Honestly, it makes me wonder if actix wouldn't be more appropriate for our use case… we would get actors and a web framework out of the box. I think we already tried to migrate once and it didn't went very far, right? What was blocking? But I'm okay with refactoring our code with Riker and keeping Rocket too.

igalic commented

2020-07-17 17:26:24 +00:00

Author

Owner

the main reasons i abandoned the actix experiment were:

it's effectively a rewrite
we're actually one of the very few (two?) ActivityPub servers not written with Actix!

(the other one isn't even public yet, so that's actually quite nice :P)

the main reasons i abandoned the [actix experiment](https://github.com/Plume-org/plume-async) were: - it's effectively a rewrite - we're actually one of the very few (two?) ActivityPub servers not written with Actix! (the other one isn't even public yet, so that's actually quite nice :P)

igalic commented

2020-07-18 18:49:12 +00:00

Author

Owner

I've created a basic example with riker and multi-type messages for how to replace Searcher: https://gist.github.com/igalic/16ec36a4c9277b3b46f6bea032226189

in a similar fashion, we can replace Worker, which currently mostly sents broadcast messages to all subscribers.

I've created a basic example with riker and multi-type messages for how to replace Searcher: https://gist.github.com/igalic/16ec36a4c9277b3b46f6bea032226189 in a similar fashion, we can replace Worker, which currently mostly sents broadcast messages to all subscribers.

👍 1

igalic commented

2020-07-21 08:52:15 +00:00

Author

Owner

here's what I've found out in an experiment attempting to extract Searcher into an Actor:

Searcher is intimately bundled with Post
despite that, Searcher is also intimately bundled with DbConn, because Post doesn't actually have all the info Searcher needs

given this asymmetry, i thought it's best to only pass Post.id

but we still have to give it access to a DbConn

perhaps on creation of a Searcher Actor, we could give it its own DbConn?

here's what I've found out in an experiment attempting to extract `Searcher` into an `Actor`: - `Searcher` is intimately bundled with `Post` - despite that, `Searcher` is also intimately bundled with `DbConn`, because `Post` doesn't actually have all the info `Searcher` needs given this asymmetry, i thought it's best to only pass `Post.id` but we still have to give it access to a `DbConn` perhaps on creation of a `Searcher` `Actor`, we could give it its own `DbConn`?

igalic referenced this issue

2020-07-22 21:15:04 +00:00

WIP: Experiment: extract Searcher into an Actor #807

igalic commented

2020-07-22 21:29:37 +00:00

Author

Owner

Some observations, which lead to the confidence with which we've started on #807:

DbPool is an Arc, it can be easily shared via .clone().
This would allow us to create a wrapper object around Searcher which can then have its own DbConn instead of passing one from a — hopefully — short-lived Request, potentially infinitely blocking on it.

The general framework lends itself nicely to be solved with an actor framework, but if that doesn't pan out, we can use any old object, and send it any old message!
We've already done good work there in extracting the essence.

It bears repeating why the refactoring to break up PlumeRocket is so important:

We pass PlumeRocket around as ref.
It's a giant struct with lots of maybe-related data, giving access to different parts of the system.
Passing it, by ref thru Rocket will become an issue as soon as we try to go async, because then we have a borrowed object sitting somewhere, waiting to be .awaited.
This is all a big mess, and best of all avoided, but it's also a good opportunity to see which parts of the system we can tear apart from there tight coupling so they will work better.

Some observations, which lead to the confidence with which we've started on #807: `DbPool` is an `Arc`, it can be easily shared via `.clone()`. This would allow us to create a wrapper object around `Searcher` which can then have its own `DbConn` instead of passing one from a — hopefully — short-lived `Request`, potentially infinitely blocking on it. The general framework lends itself nicely to be solved with an actor framework, but if that doesn't pan out, we can use any old object, and send it any old message! We've already done good work there in extracting the essence. It bears repeating why the refactoring to break up PlumeRocket is so important: We pass PlumeRocket around as `ref`. It's a giant struct with lots of maybe-related data, giving access to different parts of the system. Passing it, by `ref` thru Rocket will become an issue as soon as we try to go `async`, because then we have a borrowed object sitting somewhere, waiting to be `.await`ed. This is all a big mess, and best of all avoided, but it's also a good opportunity to see which parts of the system we can tear apart from there tight coupling so they will work better.

👍 1

igalic referenced this issue

2020-07-23 18:32:18 +00:00

plume-model: refactor Searcher to have its own DbPool #809

kiwii commented

2020-07-25 16:58:26 +00:00

Owner

Here is the list of things the worker is used for:

sending activities to other instances (just needs the activity and a list of recipients)
rotating user keypair when they delete a post (after 10 minutes), which requires the DB
fetching posts for a blog/users, either because it is new or because it has not been updated in 24 hours (depends on the DB too, and on the searcher)

Here is the list of things the worker is used for: - sending activities to other instances (just needs the activity and a list of recipients) - rotating user keypair when they delete a post (after 10 minutes), which requires the DB - fetching posts for a blog/users, either because it is new or because it has not been updated in 24 hours (depends on the DB too, and on the searcher)

igalic commented

2020-07-25 17:15:19 +00:00

Author

Owner

for the first case, i already have opened a question: https://github.com/riker-rs/riker/issues/130
and received an answer

for the first case, i already have opened a question: https://github.com/riker-rs/riker/issues/130 and received an answer

igalic referenced this issue

2020-07-26 19:36:17 +00:00

WIP: Extract DbConn, Searcher & Worker into Riker Actors #813

[Refactoring] send messages to dedicated actor instead of directly accessing objects #799

How?