tracing crate seems great! But I think we can merge this pull request. Replacing log crate with tracing after that pull request will be merged is easy: just replacing use log::
with use tracing::
.
Additionally, we are reported some ActivityPub related issues. I want to introduce logging and use it soon to investigate what's wrong. That pull request will be merged into MASTER branch, which meen we need upgrade Rocket to 0.5.0 or later to use tracing (unless we introduce it ourselves). You know, that's a hard work.
How do you think?
I respect your great research and implementation, and completely agree with introducing Riker. In addition, I make a suggestion: use Riker's Channels as a global variable. There are two points:
- The primary is to make it global, and
- The secondary is use of pub/sub model using channels
What is solved?
At current attempting (its strategy and current implementation: 8aa4ae4302), we need touch searcher more than ever before. Also, we need flow, some agents (like searcher actor or post ids) through multiple structs.
See updating search document procedure. When a user updates a post, a request handler does two (or more) things: it
- updates posts and related records in database
- sends a
UpdateDocument
message toSearcherActor
Developers need always do them together. This is needed for not only request handlers but AsObject
and FromId
implementations for models. As such, developers need write more code than now. And they might have forgotten sending message to the actor.
To make matters worse, this introduces more complexity. Assume you're deleting a user. routes::user::delete()
request handler is called. It needs to know all blogs and posts that the deleting user owns in order to send DeleteDocument
messeges corresponding to deleted posts to searcher actor. How?
One way is: Post::delete()
returns itself to Blog
, Blog::delete()
returns deleted Post
s to User
, User::delete()
returns deleted Blog
s' posts to the handler, and fanally the handler sends messeges to the actor. If some posts could not be deleted, the handler need make sure to send DeleteDocument
for each only deleted post. That's complex.
The another is: the handler passes the searcher actor to User::delete()
as an argument, User::delete()
passes it to Blog::delete()
, Blog::delete()
calls Post::delete()
and send a DeleteDocument
message to the actor several times. This is essentially same to above.
Introducing a global variable solves this. Using it, Post::insert()
, update()
and delete()
need to neither accept a searcher as an agument nor return back itself (or its ID) to the caller. What it needs to do is getting global searcher (or so) and sending a messege that it is deleted. That's it.
There are options what struct should be global like a searcher actor, an actor system or a channel. I suggest Riker's channel because...
More benefit
Use of channel introduces pub/sub model. In the case above, Post
publishes "delete" event to a channel and SearcherActor
subscribes to the event and deletes the search index corresponding to the post. More than that, we can add other actors which concern post's deletion without touching any code of request handlers and models. Examples are:
- an actor (say, "outbox") which subscribes to post's publishing, updating, deleting event and sends ActivityPub requests to other instances
- an actor ("notifier") which subscribes to post's events and updates notification page of followers and mentioned users
- similar to the above but sends notification mails
- and so on.
A proof of concept is here: https://git.joinplu.me/KitaitiMakoto/Plume/compare/igalic/refactor/extract-actors...channel. Pay your attention to
// a `SearcherActor` subscribes to any event (`"*"`)
POST_CHAN.get().unwrap().tell(Subscribe { actor: Box::new(searcher_actor), topic: "*".into() }, None);
// `Post` publishes an event without the need knowing who accepts it
if post.published {
crate::POST_CHAN.get().unwrap().tell(Publish { msg: PostUpdated(post.clone()), topic: "post.updated".into() }, None);
}
Rebasing
How do you think? I you aggree with me, I'm thiking it might be better to restart from current 0.6.0+ code (of course, I will do it). Any questions and discussions are welcome!